Added channel helpers for realtime in the client sdks #1266
Conversation
WalkthroughThis pull request adds a cross-platform Channel helper feature across multiple SDK language targets (Android, Apple, Dart, Flutter, React Native, Web). Each target receives new Channel templates that provide static methods for constructing realtime subscription channel strings with support for various resource types (database, files, teams, memberships, etc.) and optional action suffixes. Accompanying these are test templates and test updates validating the Channel helper functionality. Additionally, the Swift WebSocket implementation refactors its Channel type references to use NIOCore.Channel consistently in public APIs and internal storage. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/languages/web/node.js (1)
1-1: Critical: Channel not imported.Channel is used extensively in lines 253-273 but is never imported from the SDK. This will cause runtime errors when the tests execute.
🔎 Apply this diff to add Channel to the imports:
-const { Client, Foo, Bar, General, Query, Permission, Role, ID, Operator, Condition, MockType } = require('./dist/cjs/sdk.js'); +const { Client, Foo, Bar, General, Query, Permission, Role, ID, Channel, Operator, Condition, MockType } = require('./dist/cjs/sdk.js');tests/languages/node/test.js (1)
1-13: Critical: Channel not imported.Channel is used in lines 329-349 but is not included in the import statement. This will cause runtime errors.
🔎 Apply this diff to add Channel to the imports:
const { Client, Permission, Query, Role, ID, + Channel, Operator, Condition, MockType, Foo, Bar, General } = require('./dist/index.js');tests/languages/web/index.html (1)
24-24: Critical: Channel not destructured from Appwrite.Channel is used in lines 322-342 but is not included in the destructuring assignment. This will cause runtime errors.
🔎 Apply this diff to add Channel to the destructuring:
- const { Client, Foo, Bar, General, Realtime, Query, Permission, Role, ID, Operator, Condition, MockType } = Appwrite; + const { Client, Foo, Bar, General, Realtime, Query, Permission, Role, ID, Channel, Operator, Condition, MockType } = Appwrite;
🧹 Nitpick comments (4)
templates/dart/lib/channel.dart.twig (1)
13-24: Consider using an enum for theactionparameter for type safety.The
actionparameter accepts anyString?, but the documentation indicates only'create','update', or'delete'are valid. Consider defining an enum to enforce valid values at compile time:🔎 Example enum approach
enum ChannelAction { create, update, delete } static String database({ String databaseId = '*', String collectionId = '*', String documentId = '*', ChannelAction? action, }) { String channel = 'databases.$databaseId.collections.$collectionId.documents.$documentId'; if (action != null) { channel += '.${action.name}'; } return channel; }This would provide compile-time safety and IDE autocompletion. The current String approach is acceptable if flexibility for future action types is desired.
templates/android/library/src/main/java/io/package/Channel.kt.twig (2)
18-24: Consider using a Kotlin enum for theactionparameter.Similar to the Dart implementation, the
actionparameter accepts anyString?. Kotlin supports enums that could provide compile-time safety:🔎 Example enum approach
enum class ChannelAction { CREATE, UPDATE, DELETE; override fun toString(): String = name.lowercase() } fun database( databaseId: String = "*", collectionId: String = "*", documentId: String = "*", action: ChannelAction? = null ): String { var channel = "databases.$databaseId.collections.$collectionId.documents.$documentId" if (action != null) { channel += ".$action" } return channel }The current String approach maintains consistency with the Dart SDK if that's a design goal.
16-17: Minor: KDoc uses@return, not@returns.The KDoc standard uses
@returnfor documenting return values.@returnsis JSDoc syntax. While most tools will handle it, consider using the correct KDoc syntax for consistency.🔎 Apply this diff
- * @returns The channel string + * @return The channel stringThis applies to all methods in the class.
templates/react-native/src/channel.ts.twig (1)
1-110: Consider sharing this template with the Web SDK to reduce duplication.This file is identical to
templates/web/src/channel.ts.twig. Since both Web and React Native use TypeScript, consolidating to a single shared template would eliminate the need to maintain duplicate code.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
src/SDK/Language/Android.php(1 hunks)src/SDK/Language/Apple.php(1 hunks)src/SDK/Language/Dart.php(1 hunks)src/SDK/Language/Flutter.php(2 hunks)src/SDK/Language/ReactNative.php(1 hunks)src/SDK/Language/Web.php(1 hunks)templates/android/library/src/main/java/io/package/Channel.kt.twig(1 hunks)templates/dart/lib/channel.dart.twig(1 hunks)templates/dart/lib/package.dart.twig(1 hunks)templates/dart/test/channel_test.dart.twig(1 hunks)templates/react-native/src/channel.ts.twig(1 hunks)templates/react-native/src/index.ts.twig(1 hunks)templates/swift/Sources/Channel.swift.twig(1 hunks)templates/web/src/channel.ts.twig(1 hunks)templates/web/src/index.ts.twig(1 hunks)tests/languages/android/Tests.kt(1 hunks)tests/languages/apple/Tests.swift(1 hunks)tests/languages/flutter/tests.dart(1 hunks)tests/languages/node/test.js(1 hunks)tests/languages/swift/Tests.swift(1 hunks)tests/languages/web/index.html(1 hunks)tests/languages/web/node.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/languages/android/Tests.kt (1)
tests/languages/kotlin/Tests.kt (1)
writeToFile(271-274)
⏰ 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). (18)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, Python312)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: swift (server)
- GitHub Check: python (server)
- GitHub Check: react-native (client)
- GitHub Check: flutter (client)
- GitHub Check: apple (client)
- GitHub Check: dotnet (server)
- GitHub Check: kotlin (server)
- GitHub Check: android (client)
🔇 Additional comments (18)
tests/languages/web/node.js (1)
253-273: LGTM! Comprehensive Channel helper test coverage.The test coverage for Channel helpers is thorough, testing all methods with default parameters, specific IDs, and optional actions.
tests/languages/node/test.js (1)
329-349: LGTM! Comprehensive test coverage.The Channel helper tests are well-structured and cover all the required methods with various parameter combinations.
tests/languages/web/index.html (1)
322-342: LGTM! Thorough test coverage.The Channel helper tests are comprehensive and follow the established testing patterns.
templates/dart/test/channel_test.dart.twig (1)
1-111: LGTM! Excellent test template implementation.The test template is well-structured with:
- Proper conditional logic for Dart vs Flutter test packages
- Comprehensive coverage of all Channel methods
- Multiple test cases per method (defaults, specific IDs, actions)
- Correct use of Dart/Flutter testing conventions
src/SDK/Language/Web.php (1)
83-87: LGTM!The channel.ts file generation entry is correctly configured. The template file
templates/web/src/channel.ts.twigexists and Channel is properly exported intemplates/web/src/index.ts.twig.tests/languages/flutter/tests.dart (1)
250-270: LGTM! Comprehensive Channel helper test coverage.The tests exercise all Channel helper methods with various parameter combinations (default wildcards, specific IDs, and action suffixes), consistent with the existing test patterns in this file for Query, Permission, and Role helpers.
templates/dart/lib/channel.dart.twig (1)
45-50: Account helper correctly omits action parameter.Unlike other channel types, the account channel appropriately excludes the
actionparameter, which aligns with how account subscriptions work in Appwrite realtime.src/SDK/Language/Flutter.php (2)
83-87: LGTM! Channel file generation configuration.The channel.dart generation entry follows the established pattern for other helper files (id.dart, permission.dart, role.dart, query.dart, operator.dart).
298-302: LGTM! Channel test file generation configuration.Test file generation entry follows the same pattern as existing test entries and correctly references the shared Dart test template.
templates/web/src/channel.ts.twig (1)
14-20: Good use of TypeScript union type for action parameter.The union type
'create' | 'update' | 'delete' | nullprovides compile-time type safety, which is better than the untyped String approach in Dart. This is a good pattern.templates/web/src/index.ts.twig (1)
18-18: LGTM! Channel export added to public API.The Channel class is correctly exported, making it accessible to SDK consumers.
templates/react-native/src/index.ts.twig (1)
11-11: LGTM!The Channel export follows the existing pattern and is correctly positioned among other helper utilities.
src/SDK/Language/ReactNative.php (1)
63-67: LGTM!The Channel file generation entry follows the established pattern for other helper utilities and is correctly positioned in the file list.
src/SDK/Language/Apple.php (1)
73-77: LGTM!The Channel.swift generation entry is consistent with other Swift helper file entries and correctly positioned.
tests/languages/swift/Tests.swift (1)
238-259: Comprehensive test coverage!The Channel helper tests are well-structured and cover all methods with various parameter combinations:
- Default parameters (wildcards)
- Specific IDs
- Optional action parameters
The tests follow the existing pattern in the test suite.
src/SDK/Language/Android.php (1)
123-127: LGTM!The Channel.kt generation entry is consistent with other Android helper file entries and correctly positioned in the file list.
tests/languages/android/Tests.kt (1)
272-293: Excellent test coverage!The Channel helper tests comprehensively cover all methods with various parameter combinations and match the test coverage in other language implementations. The tests follow the existing Android test pattern using
writeToFile().templates/swift/Sources/Channel.swift.twig (1)
1-108: Well-implemented Channel helper with comprehensive documentation and consistent cross-language implementation.The implementation is clean and follows consistent patterns across all seven methods. Each method correctly builds channel strings using string interpolation and conditionally appends the optional action parameter, with support for both the traditional documents/collections terminology and the new relational model with tables/rows. All language implementations (Swift, Kotlin, TypeScript for React-Native and Web, and Dart) have identical method signatures and channel string formats, confirming consistency across the SDK generator templates.
- Updated Apple and Dart SDKs to include Channel templates. - Added Channel class in Swift template for Apple SDK. - Included Channel in Dart SDK template and test files. - Updated test cases across various platforms to incorporate Channel responses. - Modified web and node.js files to import Channel functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
templates/apple/Sources/Channel.swift.twig (1)
1-107: Consider optional action parameter validation.The documentation suggests that the action parameter should be "create", "update", or "delete", but the implementation accepts any string. While this provides flexibility, you might consider adding validation to ensure only valid actions are used, especially if the realtime API rejects invalid action values.
Example validation approach:
You could add an enum for valid actions:
public enum ChannelAction: String { case create = "create" case update = "update" case delete = "delete" }Then update method signatures to accept
ChannelAction?instead ofString?:public static func database(databaseId: String = "*", collectionId: String = "*", documentId: String = "*", action: ChannelAction? = nil) -> String { var channel = "databases.\(databaseId).collections.\(collectionId).documents.\(documentId)" if let action = action { channel += ".\(action.rawValue)" } return channel }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/SDK/Language/Apple.php(1 hunks)src/SDK/Language/Dart.php(2 hunks)templates/apple/Sources/Channel.swift.twig(1 hunks)templates/dart/test/channel_test.dart.twig(1 hunks)templates/flutter/lib/package.dart.twig(1 hunks)tests/Android14Java11Test.php(1 hunks)tests/Android14Java17Test.php(1 hunks)tests/Android14Java8Test.php(1 hunks)tests/Android5Java17Test.php(1 hunks)tests/AppleSwift56Test.php(1 hunks)tests/Base.php(1 hunks)tests/FlutterBetaTest.php(1 hunks)tests/FlutterStableTest.php(1 hunks)tests/WebChromiumTest.php(1 hunks)tests/WebNodeTest.php(1 hunks)tests/languages/android/Tests.kt(2 hunks)tests/languages/node/test.js(2 hunks)tests/languages/web/index.html(2 hunks)tests/languages/web/node.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/languages/web/node.js
- src/SDK/Language/Dart.php
- templates/dart/test/channel_test.dart.twig
🧰 Additional context used
🧬 Code graph analysis (9)
tests/WebChromiumTest.php (1)
tests/Base.php (1)
Base(17-349)
tests/Android14Java17Test.php (1)
tests/Base.php (1)
Base(17-349)
tests/AppleSwift56Test.php (1)
tests/Base.php (1)
Base(17-349)
tests/languages/android/Tests.kt (1)
tests/languages/kotlin/Tests.kt (1)
writeToFile(271-274)
tests/FlutterBetaTest.php (1)
tests/Base.php (1)
Base(17-349)
tests/WebNodeTest.php (1)
tests/Base.php (1)
Base(17-349)
tests/Android5Java17Test.php (1)
tests/Base.php (1)
Base(17-349)
tests/Android14Java8Test.php (1)
tests/Base.php (1)
Base(17-349)
tests/FlutterStableTest.php (1)
tests/Base.php (1)
Base(17-349)
⏰ 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). (20)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, Python39)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, Python312)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, Python311)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, PHP83)
- GitHub Check: build (8.3, KotlinJava8)
- GitHub Check: build (8.3, PHP80)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, CLINode16)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, DartStable)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: swift (server)
- GitHub Check: apple (client)
- GitHub Check: android (client)
🔇 Additional comments (24)
tests/languages/node/test.js (2)
7-7: LGTM!The Channel import is correctly added to the destructured require statement.
330-350: Comprehensive test coverage for Channel helpers.The test calls cover all seven Channel helper methods with appropriate parameter variations (defaults, specific IDs, and actions). The test patterns align with the expected responses defined in Base.php.
templates/apple/Sources/Channel.swift.twig (7)
11-17: LGTM!The database channel helper correctly constructs the channel string with proper parameter defaults and optional action handling.
28-34: LGTM!The tablesdb channel helper follows the same pattern as database() and correctly constructs the channel string for table-based operations.
42-44: LGTM!The account channel helper is correctly implemented with appropriate simplicity for account-based subscriptions.
54-60: LGTM!The files channel helper correctly constructs the channel string for bucket/file operations with proper action handling.
70-76: LGTM!The executions channel helper correctly constructs the channel string for function execution subscriptions.
85-91: LGTM!The teams channel helper correctly constructs the channel string with appropriate simplicity and optional action handling.
100-106: LGTM!The memberships channel helper is correctly implemented and follows the established pattern.
tests/Android5Java17Test.php (1)
35-35: LGTM!The expectedOutput array is correctly extended to include Channel helper responses, aligning with the new Channel functionality across the SDK.
tests/Android14Java11Test.php (1)
36-36: LGTM!The expectedOutput array is correctly extended to include Channel helper responses, consistent with the test data expansion across all language test suites.
tests/Android14Java17Test.php (1)
35-35: LGTM!The expectedOutput array is correctly extended to include Channel helper responses, maintaining consistency with other test files in the PR.
tests/FlutterBetaTest.php (1)
34-34: LGTM!The expectedOutput array is correctly extended to include Channel helper responses, properly aligned with the Flutter SDK's Channel helper implementation.
tests/Android14Java8Test.php (1)
36-36: LGTM!The expectedOutput array is correctly extended to include Channel helper responses, consistent with the test expectations across all Android SDK variants.
tests/WebNodeTest.php (1)
38-38: LGTM!The expectedOutput array is correctly extended to include Channel helper responses, completing the test coverage expansion for the Web SDK.
tests/AppleSwift56Test.php (1)
34-34: LGTM!The addition of
CHANNEL_HELPER_RESPONSESto the expected output is consistent with the broader PR changes adding Channel helpers across SDK languages. The placement between ID and Operator helper responses maintains a logical test ordering.src/SDK/Language/Apple.php (1)
73-77: LGTM!The Channel.swift file entry is correctly structured and positioned logically in the manifest. The use of the
apple/Sources/Channel.swift.twigtemplate path aligns with the Apple-specific implementation strategy.templates/flutter/lib/package.dart.twig (1)
33-33: LGTM!The
part 'channel.dart';directive is correctly positioned and follows proper Dart syntax for library part declarations.tests/Base.php (1)
166-187: LGTM!The
CHANNEL_HELPER_RESPONSESconstant provides comprehensive test coverage for all Channel helper methods. The patterns are well-structured with:
- Wildcard patterns for defaults (e.g.,
databases.*.collections.*.documents.*)- Specific ID patterns (e.g.,
databases.db1.collections.col1.documents.doc1)- Action-specific patterns covering create, update, and delete operations
The inclusion of "tablesdb" patterns alongside "databases" suggests backwards compatibility support, which is appropriate.
tests/languages/android/Tests.kt (1)
10-10: LGTM!The Channel import is correctly added alongside other helper imports (Permission, Role, ID, Query, Operator).
tests/WebChromiumTest.php (1)
38-38: LGTM!The addition of
CHANNEL_HELPER_RESPONSESmaintains consistency with other test files and is positioned correctly in the expected output sequence.tests/FlutterStableTest.php (1)
34-34: LGTM!The addition of
CHANNEL_HELPER_RESPONSESfollows the same pattern as other test files and maintains proper ordering in the expected output array.tests/languages/web/index.html (2)
24-24: LGTM!Channel is correctly added to the destructured imports, positioned logically alongside other helper classes (Query, Permission, Role, ID, Operator).
322-342: LGTM! Excellent cross-platform test consistency.The Channel helper tests mirror the Android implementation exactly, testing all seven methods with identical arguments:
- database, tablesdb, account, files, executions, teams, memberships
- Each with default args, specific IDs, and action-suffixed variants
The test structure demonstrates excellent consistency across SDK platforms, which will make maintenance easier and ensure uniform behavior across client libraries.
| // Channel helper tests | ||
| writeToFile(Channel.database()) | ||
| writeToFile(Channel.database("db1", "col1", "doc1")) | ||
| writeToFile(Channel.database("db1", "col1", "doc1", "create")) | ||
| writeToFile(Channel.tablesdb()) | ||
| writeToFile(Channel.tablesdb("db1", "table1", "row1")) | ||
| writeToFile(Channel.tablesdb("db1", "table1", "row1", "update")) | ||
| writeToFile(Channel.account()) | ||
| writeToFile(Channel.account("user123")) | ||
| writeToFile(Channel.files()) | ||
| writeToFile(Channel.files("bucket1", "file1")) | ||
| writeToFile(Channel.files("bucket1", "file1", "delete")) | ||
| writeToFile(Channel.executions()) | ||
| writeToFile(Channel.executions("func1", "exec1")) | ||
| writeToFile(Channel.executions("func1", "exec1", "create")) | ||
| writeToFile(Channel.teams()) | ||
| writeToFile(Channel.teams("team1")) | ||
| writeToFile(Channel.teams("team1", "create")) | ||
| writeToFile(Channel.memberships()) | ||
| writeToFile(Channel.memberships("membership1")) | ||
| writeToFile(Channel.memberships("membership1", "update")) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Tests.kt file in the repository
find . -name "Tests.kt" -path "*/android/*" 2>/dev/null | head -20Repository: appwrite/sdk-generator
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Read the Tests.kt file to verify the test code and examine the assertions
head -300 ./tests/languages/android/Tests.kt | tail -50
# Also find the Channel.kt template
find . -name "*.twig" -path "*android*" -name "*channel*" -o -name "*.twig" -path "*android*" -name "*Channel*" 2>/dev/null | head -10Repository: appwrite/sdk-generator
Length of output: 2884
🏁 Script executed:
#!/bin/bash
# Examine the Channel.kt template to verify default parameter support
cat ./templates/android/library/src/main/java/io/package/Channel.kt.twig | head -150Repository: appwrite/sdk-generator
Length of output: 4249
🏁 Script executed:
#!/bin/bash
# Find and examine Base.php to verify CHANNEL_HELPER_RESPONSES
find . -name "Base.php" -path "*/tests/*" 2>/dev/nullRepository: appwrite/sdk-generator
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Examine Base.php to find CHANNEL_HELPER_RESPONSES
grep -A 50 "CHANNEL_HELPER_RESPONSES" ./tests/Base.phpRepository: appwrite/sdk-generator
Length of output: 2248
Correct test case count to 20, not 21.
The Channel helper tests provide comprehensive coverage of all seven methods with proper default parameter support. The 20 test cases (not 21) systematically exercise:
- Default parameters (wildcards):
Channel.database(),Channel.account(), etc. - Specific IDs:
Channel.database("db1", "col1", "doc1"),Channel.account("user123") - Specific IDs with actions:
Channel.database("db1", "col1", "doc1", "create")
All test patterns align perfectly with the 20 expected responses in CHANNEL_HELPER_RESPONSES defined in tests/Base.php, and the generated Channel.kt template correctly implements default parameter support using Kotlin's default argument syntax with wildcard defaults and nullable optional parameters.
🤖 Prompt for AI Agents
In tests/languages/android/Tests.kt around lines 273 to 293, the Channel helper
section generates 20 test cases but the test expectation is set to 21; update
the expectation to 20 so it matches the actual generated calls. Locate where the
test-count or expected responses for CHANNEL_HELPER_RESPONSES (or the assertion
that checks for 21 entries) is set in this file or the related test harness and
change the value from 21 to 20 so the asserted count matches the 20 writeToFile
calls.
There was a problem hiding this comment.
Its correct. Otherwise It would fail the tests
There was a problem hiding this comment.
@ArnabChatterjee20k, you're absolutely right, and I apologize for the confusion. I've recounted the test cases and confirmed there are exactly 20 Channel helper tests in the code, which correctly matches the 20 expected responses defined in CHANNEL_HELPER_RESPONSES in tests/Base.php. My previous comment about needing to update from 21 to 20 was incorrect. The tests are properly aligned and would indeed fail if the counts didn't match. Thank you for the clarification! 👍
…Channel helper tests from node and swift
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
templates/apple/Sources/Channel.swift.twig (2)
13-19: Consider validating the action parameter.The documentation specifies that
actionshould be "create", "update", or "delete", but there's no validation to enforce this. While the server likely validates channel strings, client-side validation would provide better developer experience by failing fast with clear error messages rather than silently creating invalid subscriptions.🔎 View suggested validation approach:
You could add validation in each method, for example in the
databasemethod:public static func database(databaseId: String = "*", collectionId: String = "*", documentId: String = "*", action: String? = nil) -> String { + if let action = action, !["create", "update", "delete"].contains(action) { + preconditionFailure("Invalid action '\(action)'. Must be 'create', 'update', or 'delete'.") + } var channel = "databases.\(databaseId).collections.\(collectionId).documents.\(documentId)" if let action = action { channel += ".\(action)" } return channel }Apply similar validation to the other methods with action parameters.
Also applies to: 30-36, 56-62, 72-78, 87-93, 102-108
4-12: Consider using Swift-standard documentation syntax.The documentation uses
@paramand@returns, which are not standard Swift documentation markers. Swift documentation typically uses- Parameter(or- Parameters:) and- Returns:for better integration with Xcode and documentation generation tools like jazzy.🔎 Example with Swift-standard syntax:
/** * Generate a database channel string. * - * @param databaseId The database ID (default: "*") - * @param collectionId The collection ID (default: "*") - * @param documentId The document ID (default: "*") - * @param action Optional action: "create", "update", or "delete" (default: nil) - * @returns The channel string + * - Parameter databaseId: The database ID (default: "*") + * - Parameter collectionId: The collection ID (default: "*") + * - Parameter documentId: The document ID (default: "*") + * - Parameter action: Optional action: "create", "update", or "delete" (default: nil) + * - Returns: The channel string */Also applies to: 21-29, 38-43, 48-55, 64-71, 80-86, 95-101
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
templates/apple/Sources/Channel.swift.twig(1 hunks)templates/swift/Sources/WebSockets/WebSocketClient.swift.twig(7 hunks)templates/swift/Sources/WebSockets/WebSocketClientDelegate.swift.twig(1 hunks)
⏰ 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). (19)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, WebNode)
- GitHub Check: apple (client)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, Ruby27)
- GitHub Check: build (8.3, Python311)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: kotlin (server)
- GitHub Check: build (8.3, KotlinJava8)
- GitHub Check: build (8.3, Node20)
- GitHub Check: android (client)
- GitHub Check: build (8.3, DotNet80)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: swift (server)
- GitHub Check: build (8.3, CLINode18)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, KotlinJava11)
🔇 Additional comments (4)
templates/swift/Sources/WebSockets/WebSocketClient.swift.twig (3)
35-43: Property type qualification is correct and thread-safe.The channel property and its backing field
_channelhave been consistently updated to useNIOCore.Channel?. The synchronized access pattern usingchannelQueueensures thread safety.
64-90: Callback storage updates are consistent and thread-safe.The callback storage for
onOpenandonClosehas been properly updated to useNIOCore.Channelin their function signatures. The synchronization using thelockerqueue ensures thread-safe access to the callback storage.
265-265: Internal method signatures correctly updated.The private methods
openChannelandupgradePipelineHandlerhave been updated to useNIOCore.Channel, maintaining internal consistency with the property types and callback signatures. The delegate call at line 302 will work correctly since the protocol has been updated accordingly.Also applies to: 293-293
templates/swift/Sources/WebSockets/WebSocketClientDelegate.swift.twig (1)
7-7: Consistent namespace qualification to prevent naming conflicts.The refactoring from
ChanneltoNIOCore.Channelis consistent across the protocol declaration and default extension implementations. NIO is an umbrella module exporting NIOCore, NIOEmbedded and NIOPosix, soNIOCore.Channelis accessible via the existingimport NIOon line 2. All four method signatures (lines 7, 10, 16, 22) are properly updated. This is a breaking change to the public API but necessary for avoiding type name collisions with new Channel helpers for realtime subscriptions.
| public static func database(databaseId: String = "*", collectionId: String = "*", documentId: String = "*", action: String? = nil) -> String { | ||
| var channel = "databases.\(databaseId).collections.\(collectionId).documents.\(documentId)" | ||
| if let action = action { | ||
| channel += ".\(action)" | ||
| } | ||
| return channel | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for ID validation or sanitization in the template codebase
fd -e twig . -type f | head -20Repository: appwrite/sdk-generator
Length of output: 236
🏁 Script executed:
# Check for any ID format validation patterns in templates
rg -i "alphanumeric|valid.*id|id.*constraint" --type-list | head -5Repository: appwrite/sdk-generator
Length of output: 455
🌐 Web query:
Appwrite ID format constraints valid characters specifications
💡 Result:
Summary of Appwrite ID format rules:
- Allowed characters: lowercase letters a–z, uppercase A–Z, digits 0–9, period (.), hyphen (-), underscore (_). [1]
- Cannot start with a special character (., -, _). [1]
- Maximum length: 36 characters. [1]
- You can provide a custom ID or use the SDK helper ID.unique() / pass "unique()" when using REST. [2]
Sources: [1] Appwrite Users API reference; [2] Appwrite REST / Unique ID documentation.
🏁 Script executed:
# First, find and read the Channel.swift.twig file
fd "Channel.swift.twig" -type fRepository: appwrite/sdk-generator
Length of output: 236
🏁 Script executed:
# Search for any sanitization, validation, or escaping logic in template files
rg "sanitize|escape|validate|replace.*\\\." -i templates/Repository: appwrite/sdk-generator
Length of output: 50378
🏁 Script executed:
# Check other language channel implementations for comparison
fd "Channel.*twig" -type f | head -10Repository: appwrite/sdk-generator
Length of output: 236
🏁 Script executed:
# Find the Channel.swift.twig file
fd "Channel.swift.twig"Repository: appwrite/sdk-generator
Length of output: 108
🏁 Script executed:
# Read the Channel.swift.twig file
cat templates/apple/Sources/Channel.swift.twigRepository: appwrite/sdk-generator
Length of output: 3893
🏁 Script executed:
# Search for any ID validation or escaping in Swift templates
rg "sanitize|escape|validate" templates/apple/ -iRepository: appwrite/sdk-generator
Length of output: 892
Add ID validation or escaping to prevent malformed channel strings.
Appwrite allows IDs to contain periods, hyphens, and underscores. The methods currently interpolate these IDs directly into dot-separated channel strings without validation or escaping. If an ID contains a period (e.g., databaseId="my.db"), it produces an ambiguous channel string like "databases.my.db.collections.*..." that cannot be reliably parsed. Either validate that IDs don't contain delimiter characters, or escape/encode IDs before interpolation.
Also applies to: 30-36 (tablesdb), 44-46 (account), 56-62 (files), 72-78 (executions), 87-93 (teams), 102-108 (memberships)
🤖 Prompt for AI Agents
In templates/apple/Sources/Channel.swift.twig around lines 13 to 19 (and
similarly for lines 30-36, 44-46, 56-62, 72-78, 87-93, 102-108), the code
interpolates IDs directly into dot-delimited channel strings which breaks when
IDs contain delimiter characters (like periods). Fix by validating or
normalizing IDs before interpolation: either reject IDs containing delimiters
and throw/return an error, or consistently escape/encode IDs (e.g.,
percent-encode or a reversible encoding) and use the encoded form in the channel
string so parsing remains unambiguous; apply the same validation/encoding logic
to all listed channel factory functions and update their tests accordingly.
There was a problem hiding this comment.
We dont follow this in sdk. It is just an helper similar to Query
There was a problem hiding this comment.
@ArnabChatterjee20k, thank you for the clarification! I understand that the Channel helper follows the same pattern as Query and other helpers in the SDK, where IDs are accepted as-is without validation or escaping. This is consistent with your SDK's design philosophy of providing lightweight helper utilities.
I've noted this pattern for future reviews.
✏️ Learnings added
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/apple/Sources/Channel.swift.twig:13-19
Timestamp: 2025-12-18T18:43:30.593Z
Learning: In the appwrite/sdk-generator repository, helper classes like Channel and Query accept IDs without validation or escaping. These are lightweight helper utilities that construct strings directly from provided parameters without sanitizing or encoding them. This is an intentional design pattern across the SDK.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/swift/Sources/WebSockets/WebSocketClient.swift.twig:140-140
Timestamp: 2025-12-18T18:42:14.446Z
Learning: In templates/swift/Sources/WebSockets/WebSocketClient.swift.twig, the Channel type from NIOCore is explicitly qualified as `NIOCore.Channel` to avoid naming conflicts with the new Channel helper class for realtime subscriptions. This is not a breaking change - it's the same type, just explicitly qualified in the source.
What does this PR do?
(Provide a description of what this PR does.)
eg; Channel::database(databaseId: string = '', collectionId: string = '', documentId: string = '*', action: create|update|delete)
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
Tests
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.