Added channel helpers for realtime in the client sdks #1266
Conversation
WalkthroughThis PR adds comprehensive Channel builder functionality across the Appwrite SDK for multiple languages (Kotlin/Android, Swift/Apple, Dart/Flutter, TypeScript/React Native, TypeScript/Web). New template files generate language-specific Channel classes that construct realtime subscription channel strings using fluent builder patterns with default wildcard values. Realtime subscription APIs are updated to accept both string channel names and Channel builder instances. The Swift WebSocket client is refactored to use NIOCore.Channel types instead of a generic Channel type. Test coverage is expanded with Channel helper tests and a new test data structure in the base test class. Estimated code review effort🎯 4 (Complex) | ⏱️ ~90 minutes Areas requiring extra attention
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)🚧 Files skipped from review as they are similar to previous changes (1)🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-12-18T18:42:20.227ZApplied to files: 🔇 Additional comments (7)
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 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.
| export class Channel { | ||
| /** | ||
| * Generate a database channel string. | ||
| * | ||
| * @param {string} databaseId | ||
| * @param {string} collectionId | ||
| * @param {string} documentId | ||
| * @param {'create'|'update'|'delete'|null} action | ||
| * @returns {string} | ||
| */ | ||
| static database(databaseId: string = '*', collectionId: string = '*', documentId: string = '*', action: 'create' | 'update' | 'delete' | null = null): string { | ||
| let channel = `databases.${databaseId}.collections.${collectionId}.documents.${documentId}`; | ||
| if (action) { | ||
| channel += `.${action}`; | ||
| } | ||
| return channel; | ||
| } |
There was a problem hiding this comment.
Let's change the style a bit more like a builder instead, so we can do it like Channel.database('db').table('tb').row('*').create()
ArnabChatterjee20k
force-pushed
the
dat-971
branch
from
693fe53 to
fd57b65
Compare
There was a problem hiding this comment.
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/apple/Sources/Services/Realtime.swift.twig (1)
254-254: QualifyChannelparameter asNIOCore.Channelin delegate method to avoid naming conflict.The
onOpen(channel: Channel)andonClose(channel: Channel, ...)delegate methods in Realtime.swift.twig (line 254+) use an unqualifiedChanneltype. Since Channel.swift.twig defines a newChannelhelper class for subscription channels, this creates ambiguity. The delegate parameter should explicitly useNIOCore.Channelto match the pattern in WebSocketClient.swift.twig (which consistently qualifies all Channel references asNIOCore.Channel), ensuring the correct network channel type is referenced and avoiding resolution to the subscription helper class.
🧹 Nitpick comments (10)
tests/Base.php (3)
283-290: Consider removing verbose debug logging before merging.The extensive
[LOG]statements throughout this method appear to be debug instrumentation. If these were added to troubleshoot specific CI issues, consider removing them before merging to keep test output clean, or wrap them in a conditional flag (e.g.,$verboseor environment variable check) if you want them available on-demand.
295-322: Duplicate build directory cleanup logic.The build directory is cleaned twice with identical logic (lines 295-300 and 317-322). If both cleanups are truly necessary to avoid PCH cache issues, consider extracting this into a helper method to reduce duplication.
🔎 Proposed refactor
+ private function cleanBuildDirectory(string $context = ''): void + { + $buildDir = __DIR__ . '/sdks/' . $this->language . '/.build'; + if (is_dir($buildDir)) { + echo "[LOG] Cleaning build directory{$context}: {$buildDir}\n"; + $this->rmdirRecursive($buildDir); + } + }Then replace both cleanup blocks with:
$this->cleanBuildDirectory(); // ... build commands ... $this->cleanBuildDirectory(' before test');
364-366: Minor: Usecount()for consistency.The file uses
count()everywhere else (lines 332, 341, 354, 363, 370, 371, 405), butsizeof()here. While functionally equivalent, prefercount()for consistency.- } while ($removed != 'Test Started' && sizeof($output) != 0); + } while ($removed != 'Test Started' && count($output) != 0);templates/flutter/lib/src/realtime_mixin.dart.twig (1)
171-177: Consider documenting expected channel types.The
_channelToStringhelper correctly converts channel values to strings. However, since the parameter isObject, any value could be passed at runtime (in Dart, type checking is not always enforced). Consider adding a doc comment explaining that channels should beStringor implementtoString()meaningfully.💡 Optional: Add documentation
+ /// Convert channel value to string. + /// Accepts String or Channel builder instances that implement toString(). String _channelToString(Object channel) { if (channel is String) { return channel; } return channel.toString(); }templates/web/src/services/realtime.ts.twig (2)
248-257: Simplify thechannelToStringhelper.The
toStringmethod exists on all JavaScript objects, making the explicit check on lines 253-254 and the fallback on line 256 redundant. The helper can be simplified.🔎 Suggested simplification
private channelToString(channel: string | ChannelValue): string { if (typeof channel === 'string') { return channel; } - // Channel builder instances have toString() method - if (channel && typeof channel.toString === 'function') { - return channel.toString(); - } - return String(channel); + return channel.toString(); }
266-269: Type signature mismatch with JSDoc.The JSDoc on line 262 states the parameter accepts "string or Channel builder instance", but the type signature only allows
ChannelValue. For consistency with the array overloads and the documentation, consider acceptingstring | ChannelValue.🔎 Proposed fix
public async subscribe( - channel: ChannelValue, + channel: string | ChannelValue, callback: (event: RealtimeResponseEvent<any>) => void ): Promise<RealtimeSubscription>;Apply the same change to the generic single-channel overload on line 291:
public async subscribe<T>( - channel: ChannelValue, + channel: string | ChannelValue, callback: (event: RealtimeResponseEvent<T>) => void ): Promise<RealtimeSubscription>;templates/android/library/src/main/java/io/package/Channel.kt.twig (1)
179-188: Consider consistency:accountreturnsStringwhile other methods return channel builders.The
account()method returns aStringdirectly, unlike other companion methods which return channel builder objects. This works but creates an inconsistent API surface. If intentional (since account has no sub-resources), consider adding a brief doc note explaining this difference.templates/apple/Sources/Channel.swift.twig (2)
46-64: Consider removing unused stored properties.The
databaseIdandcollectionIdproperties are stored but only used during initialization to constructbase. If they're not needed for future extensions, they could be removed to reduce memory footprint.🔎 Proposed refactor
public class CollectionChannel { private let base: String - private let databaseId: String - private let collectionId: String init(databaseId: String, collectionId: String) { - self.databaseId = databaseId - self.collectionId = collectionId self.base = "databases.\(databaseId).collections.\(collectionId)" }
192-200: Consider using Swift-style documentation comments.The current documentation uses Java-style
@param/@returnsannotations. Swift convention uses- Parameter:and- Returns:with triple-slash comments.🔎 Swift-idiomatic documentation example
- /** - * Generate a database channel builder. - * - * @param databaseId The database ID (default: "*") - * @returns DatabaseChannel - */ + /// Generate a database channel builder. + /// + /// - Parameter databaseId: The database ID (default: "*") + /// - Returns: A `DatabaseChannel` instance public static func database(_ databaseId: String = "*") -> DatabaseChannel {templates/flutter/lib/channel.dart.twig (1)
181-181: Consider ifChannelValuetype alias provides sufficient type safety.The
typedef ChannelValue = Objectis very broad and doesn't constrain what can be passed. While Dart lacks TypeScript's union types, you could consider a marker interface or sealed class if stronger type safety is desired. However, this matches the SDK's lightweight helper philosophy.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/SDK/Language/Flutter.php(2 hunks)templates/android/library/src/main/java/io/package/Channel.kt.twig(1 hunks)templates/android/library/src/main/java/io/package/services/Realtime.kt.twig(2 hunks)templates/apple/Sources/Channel.swift.twig(1 hunks)templates/apple/Sources/Services/Realtime.swift.twig(3 hunks)templates/flutter/lib/channel.dart.twig(1 hunks)templates/flutter/lib/src/realtime.dart.twig(1 hunks)templates/flutter/lib/src/realtime_base.dart.twig(1 hunks)templates/flutter/lib/src/realtime_browser.dart.twig(1 hunks)templates/flutter/lib/src/realtime_io.dart.twig(1 hunks)templates/flutter/lib/src/realtime_mixin.dart.twig(1 hunks)templates/web/src/channel.ts.twig(1 hunks)templates/web/src/client.ts.twig(3 hunks)templates/web/src/services/realtime.ts.twig(2 hunks)tests/Base.php(2 hunks)tests/languages/android/Tests.kt(2 hunks)tests/languages/apple/Tests.swift(1 hunks)tests/languages/flutter/tests.dart(1 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 (2)
- tests/languages/web/index.html
- src/SDK/Language/Flutter.php
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
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.
📚 Learning: 2025-12-18T18:42:14.446Z
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.
Applied to files:
templates/flutter/lib/src/realtime_io.dart.twigtemplates/flutter/lib/src/realtime_mixin.dart.twigtemplates/web/src/client.ts.twigtemplates/android/library/src/main/java/io/package/services/Realtime.kt.twigtemplates/flutter/lib/src/realtime_base.dart.twigtemplates/flutter/lib/src/realtime.dart.twigtemplates/apple/Sources/Channel.swift.twigtests/languages/apple/Tests.swifttemplates/flutter/lib/channel.dart.twigtemplates/flutter/lib/src/realtime_browser.dart.twigtemplates/android/library/src/main/java/io/package/Channel.kt.twigtemplates/apple/Sources/Services/Realtime.swift.twigtemplates/web/src/channel.ts.twigtemplates/web/src/services/realtime.ts.twig
📚 Learning: 2025-12-18T18:43:30.593Z
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.
Applied to files:
tests/languages/web/node.jstests/languages/flutter/tests.darttemplates/apple/Sources/Channel.swift.twigtests/languages/apple/Tests.swifttemplates/flutter/lib/channel.dart.twigtemplates/android/library/src/main/java/io/package/Channel.kt.twigtemplates/web/src/channel.ts.twigtests/languages/android/Tests.kt
🧬 Code graph analysis (1)
tests/languages/android/Tests.kt (1)
tests/languages/kotlin/Tests.kt (1)
writeToFile(271-274)
🔇 Additional comments (36)
tests/languages/android/Tests.kt (2)
10-10: LGTM!The import for the new
Channelhelper class is correctly added alongside the existing helper imports.
273-293: Comprehensive Channel helper test coverage.The 20 test cases systematically exercise all Channel helper methods with appropriate variations:
- Default wildcards (e.g.,
Channel.database().collection().document())- Specific IDs (e.g.,
Channel.database("db1").collection("col1").document("doc1"))- Action methods (e.g.,
.create(),.update(),.delete())The fluent builder API is well-tested across
database,tablesdb,buckets,functions,teams, andmemberships, whileaccountcorrectly returns a String directly. Based on learnings, the design intentionally accepts IDs without validation, consistent with other SDK helpers like Query.tests/Base.php (1)
166-188: LGTM!The
CHANNEL_HELPER_RESPONSESconstant is well-structured with 20 entries covering all the channel helper patterns (databases, tables, buckets, functions, teams, memberships, account). The patterns follow a consistent naming convention and include variations with wildcards, specific IDs, and action suffixes.templates/flutter/lib/src/realtime_io.dart.twig (1)
46-48: LGTM! Signature broadened to support Channel builders.The method signature correctly accepts
List<Object>to support both string channels and Channel builder instances, delegating conversion logic to the mixin.tests/languages/flutter/tests.dart (1)
250-271: LGTM! Comprehensive Channel helper test coverage.The tests appropriately exercise the Channel builder API across all resource types (database, tablesdb, account, buckets, functions, teams, memberships) with various parameter combinations and action modifiers.
templates/flutter/lib/src/realtime_browser.dart.twig (1)
38-40: LGTM! Consistent signature update for browser platform.The signature change matches the IO platform implementation, correctly delegating to the mixin for channel conversion.
templates/flutter/lib/src/realtime_base.dart.twig (1)
6-6: LGTM! Abstract contract correctly broadened.The abstract method signature properly defines the contract for platform implementations to accept both string and Channel builder types.
tests/languages/apple/Tests.swift (1)
251-271: LGTM! Channel helper tests for Apple SDK.The tests comprehensively exercise the Channel builder API with appropriate coverage of all resource types and action variants. The previous concern about Channel class availability was addressed in commit 3f13cb1.
templates/flutter/lib/src/realtime.dart.twig (1)
45-53: LGTM! Clear documentation with practical Channel builder examples.The updated signature and documentation effectively demonstrate how to use Channel builders alongside traditional string channels. The examples are practical and show mixed usage patterns.
templates/web/src/client.ts.twig (1)
585-613: LGTM! Robust channel type normalization logic.The implementation correctly handles the union type
string | string[] | ChannelValue | ChannelValue[]by:
- Normalizing to array form
- Converting each channel to string via
toString()method orString()fallback- Maintaining internal consistency with string-based channel storage
The defensive
typeof ch.toString === 'function'check ensures safe handling of edge cases, and the documentation clearly demonstrates Channel builder usage.templates/flutter/lib/src/realtime_mixin.dart.twig (1)
179-201: LGTM! Clean channel type normalization in subscription logic.The implementation correctly:
- Accepts
List<Object>to support both strings and Channel builders- Normalizes all channels to strings using
_channelToString- Maintains string-based internal storage for consistency
- Uses
channelStringsthroughout for subscription tracking and cleanuptemplates/web/src/services/realtime.ts.twig (1)
311-317: LGTM!The implementation correctly normalizes input to an array, converts all channels to strings, and creates a Set for deduplication. The logic integrates well with the existing subscription management.
templates/android/library/src/main/java/io/package/Channel.kt.twig (3)
1-7: LGTM!The
ResolvedChannelwrapper is a clean approach for representing finalized channel strings with action suffixes.
9-25: LGTM!The
ActionChannelabstract class provides a clean abstraction for channels that support CRUD action suffixes. The pattern correctly separates the base channel path from action qualifiers.
27-54: LGTM!The database channel hierarchy correctly models the Appwrite resource structure with a fluent API:
Channel.database().collection().document().tests/languages/web/node.js (2)
1-1: LGTM!The
Channelimport is correctly added alongside other helper classes.
253-273: LGTM!Comprehensive test coverage for the Channel helper API. Tests verify:
- Default wildcard parameters
- Specific ID parameters
- Nested path construction (database → collection → document)
- Action methods (create, update, delete)
- All channel types (database, tablesdb, account, buckets, functions, teams, memberships)
templates/android/library/src/main/java/io/package/services/Realtime.kt.twig (2)
112-120: LGTM!The
channelToStringhelper is clean and idiomatic Kotlin. Using awhenexpression provides clear handling for String passthrough and fallback totoString()for other types.
122-150: LGTM!The new overloads correctly extend the API to accept channel builder objects while maintaining backward compatibility with existing String-based signatures. The delegation pattern keeps the implementation DRY.
templates/apple/Sources/Services/Realtime.swift.twig (3)
6-17: LGTM!The
ChannelValueprotocol is a clean Swift idiom for abstracting channel types. ExtendingStringto conform allows seamless backward compatibility with existing string-based API usage.
147-198: LGTM!The subscribe method overloads are well-structured:
channelToStringhelper encapsulates the conversion- Public API accepts
ChannelValuefor flexibility- Internal implementation uses
Set<String>for deduplication- All overloads correctly delegate to the core implementation
19-31: No action required. All channel types shown in the extensions already implement thetoString() -> Stringmethod inChannel.swift.twig, confirming the empty extensions toChannelValueprotocol are valid.templates/apple/Sources/Channel.swift.twig (4)
1-37: LGTM!The
ResolvedChannelwrapper andActionChannelbase class are well-structured. The use ofopen classforActionChannelappropriately allows subclassing, and the fluentcreate()/update()/delete()methods provide a clean API.
84-127: LGTM!The TablesDB hierarchy follows the same consistent builder pattern as the Database hierarchy. The implementation is correct and enables fluent channel construction like
Channel.tablesdb("db").table("tbl").row("*").create().
129-177: LGTM!The Buckets and Functions hierarchies are correctly implemented following the established builder pattern.
179-189: LGTM!Simple and correct implementation for team and membership channels as leaf-level builders.
templates/flutter/lib/channel.dart.twig (4)
1-34: LGTM!Clean Dart implementation of the base channel classes. Using
abstract classforActionChannelis appropriate, and the@overrideannotations ontoString()are correct since all Dart classes extendObject.
36-77: LGTM!The Database → Collection → Document hierarchy is well-implemented using Dart's initializer list syntax. The builder pattern enables fluent channel construction.
79-170: LGTM!Consistent implementation across all resource hierarchies (TablesDB, Buckets, Functions) following the established builder pattern.
183-239: LGTM!The
Channelfacade is well-implemented with proper Dart-style documentation comments. The static factory methods provide a clean public API for channel construction.templates/web/src/channel.ts.twig (6)
1-37: LGTM!The base channel classes are well-implemented.
ResolvedChannelis appropriately exported whileActionChannelremains internal. The empty lines at the start of the file (lines 1-2) appear intentional for template rendering.
39-81: LGTM!Good use of TypeScript's parameter property shorthand (
private databaseId: string). The non-exported classes provide proper encapsulation, forcing users to access them through theChannelfacade.
83-175: LGTM!Consistent implementation across TablesDB, Buckets, and Functions hierarchies. The builder pattern enables fluent construction like
Channel.tablesdb('db').table('tb').row('*').create().
177-187: LGTM!Simple and correct implementation for team and membership channels.
189-194: LGTM!The
ChannelValueunion type provides excellent compile-time type safety, allowing consumers to use any channel type or raw strings.
196-266: LGTM!The
Channelfacade provides a clean public API with well-documented static factory methods. The builder pattern implementation addresses the style requested in previous reviews, enabling fluent construction likeChannel.database('db').collection('col').document('doc').create().
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
templates/flutter/lib/src/realtime_mixin.dart.twig (1)
179-201: Type broadening implemented correctly.The changes successfully broaden the API to accept
List<Object>channels and convert them consistently to strings for internal processing. The conversion is done upfront and applied uniformly throughout the subscription lifecycle.Optional: Remove redundant cast on line 181.
Since
_channelToStringreturnsString, the.map()operation already produces anIterable<String>, and.toList()yields aList<String>. The.cast<String>()call is redundant.🔎 Suggested simplification
- final channelStrings = channels.map((ch) => _channelToString(ch)).toList().cast<String>(); + final channelStrings = channels.map((ch) => _channelToString(ch)).toList();templates/apple/Sources/Services/Realtime.swift.twig (1)
147-152: Consider inlining or removing this helper.The
channelToStringmethod is a trivial wrapper aroundchannel.toString(). It could be inlined at call sites (e.g.,channels.map { $0.toString() }), reducing indirection. However, keeping it is acceptable if you prefer a single conversion point for maintainability.templates/web/src/client.ts.twig (1)
556-612: Well-implemented ChannelValue support in subscribe method.The implementation properly extends the subscribe method to accept both string and ChannelValue types, with clean normalization logic that converts Channel builder instances to strings via toString(). The fallback handling and documentation are appropriate.
💡 Optional: Extract channel normalization helper
The channel normalization logic (lines 588-596) could be extracted to a private helper method similar to the pattern used in templates/web/src/services/realtime.ts.twig (lines 248-257). This would improve consistency and reusability, though the current inline implementation is perfectly acceptable.
+ private channelToString(channel: string | ChannelValue): string { + if (typeof channel === 'string') { + return channel; + } + if (channel && typeof channel.toString === 'function') { + return channel.toString(); + } + return String(channel); + } + subscribe<T extends unknown>(channels: string | string[] | ChannelValue | ChannelValue[], callback: (payload: RealtimeResponseEvent<T>) => void): () => void { const channelArray = Array.isArray(channels) ? channels : [channels]; - // Convert Channel instances to strings - const channelStrings = channelArray.map(ch => { - if (typeof ch === 'string') { - return ch; - } - // Channel builder instances have toString() method - if (ch && typeof ch.toString === 'function') { - return ch.toString(); - } - return String(ch); - }); + const channelStrings = channelArray.map(ch => this.channelToString(ch));
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/SDK/Language/Flutter.php(2 hunks)templates/android/library/src/main/java/io/package/Channel.kt.twig(1 hunks)templates/android/library/src/main/java/io/package/services/Realtime.kt.twig(2 hunks)templates/apple/Sources/Channel.swift.twig(1 hunks)templates/apple/Sources/Services/Realtime.swift.twig(3 hunks)templates/flutter/lib/channel.dart.twig(1 hunks)templates/flutter/lib/src/realtime.dart.twig(1 hunks)templates/flutter/lib/src/realtime_base.dart.twig(1 hunks)templates/flutter/lib/src/realtime_browser.dart.twig(1 hunks)templates/flutter/lib/src/realtime_io.dart.twig(1 hunks)templates/flutter/lib/src/realtime_mixin.dart.twig(1 hunks)templates/web/src/channel.ts.twig(1 hunks)templates/web/src/client.ts.twig(3 hunks)templates/web/src/services/realtime.ts.twig(2 hunks)tests/languages/android/Tests.kt(2 hunks)tests/languages/apple/Tests.swift(1 hunks)tests/languages/flutter/tests.dart(1 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 (7)
- templates/flutter/lib/src/realtime_io.dart.twig
- templates/flutter/lib/src/realtime_base.dart.twig
- tests/languages/apple/Tests.swift
- src/SDK/Language/Flutter.php
- templates/web/src/channel.ts.twig
- templates/flutter/lib/channel.dart.twig
- templates/flutter/lib/src/realtime.dart.twig
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
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.
📚 Learning: 2025-12-18T18:42:14.446Z
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.
Applied to files:
templates/flutter/lib/src/realtime_browser.dart.twigtemplates/web/src/client.ts.twigtemplates/flutter/lib/src/realtime_mixin.dart.twigtemplates/android/library/src/main/java/io/package/Channel.kt.twigtemplates/android/library/src/main/java/io/package/services/Realtime.kt.twigtemplates/web/src/services/realtime.ts.twigtemplates/apple/Sources/Services/Realtime.swift.twigtemplates/apple/Sources/Channel.swift.twig
📚 Learning: 2025-12-18T18:43:30.593Z
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.
Applied to files:
tests/languages/flutter/tests.darttests/languages/web/node.jstemplates/android/library/src/main/java/io/package/Channel.kt.twigtests/languages/web/index.htmltests/languages/android/Tests.kttemplates/apple/Sources/Channel.swift.twig
🧬 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). (14)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, Go118)
- GitHub Check: build (8.3, Node20)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, Go112)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, KotlinJava8)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, CLINode18)
- GitHub Check: build (8.3, CLINode16)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: android (client)
- GitHub Check: swift (server)
- GitHub Check: apple (client)
🔇 Additional comments (25)
templates/flutter/lib/src/realtime_browser.dart.twig (1)
38-40: LGTM! Type broadening aligns with Channel helper support.The signature change from
List<String>toList<Object>allows this method to accept Channel helper objects alongside plain strings, which is the core objective of this PR. The delegation tosubscribeToremains unchanged.templates/flutter/lib/src/realtime_mixin.dart.twig (1)
171-177: LGTM! Clean conversion helper.The
_channelToStringhelper appropriately handles both string and non-string channel values by delegating to.toString()for Channel objects. This aligns with the established pattern for helper classes in this SDK.templates/android/library/src/main/java/io/package/services/Realtime.kt.twig (3)
112-120: LGTM! Clean channel conversion helper.The
channelToStringimplementation correctly handles both String and other channel types (like Channel helper objects). The String check is a good optimization to avoid unnecessarytoString()calls.As per learnings, no validation or escaping is performed on IDs, which aligns with the intentional design pattern across the SDK.
122-129: LGTM! Backward-compatible channel helper support.This overload cleanly enables developers to pass Channel helper objects (or other types) to
subscribe()while maintaining full backward compatibility. The delegation to the existing String-based implementation ensures consistent behavior.Kotlin's type resolution will prefer the
Stringoverload when strings are passed directly, and thisAnyoverload when Channel objects are used.
140-150: LGTM! Type-safe channel helper integration.This typed overload correctly extends the Channel helper support to include custom payload types while preserving type safety. The delegation pattern cleanly maps channels to strings and forwards to the existing typed implementation, properly returning the
RealtimeSubscriptionhandle.This allows developers to write type-safe subscriptions like:
realtime.subscribe( Channel.database("db123", "col456"), payloadType = MyDocument::class.java ) { event -> // event.payload is MyDocument }templates/apple/Sources/Services/Realtime.swift.twig (7)
6-11: LGTM!The
ChannelValueprotocol is clean and minimal. UsingtoString()rather than Swift'sCustomStringConvertibleis acceptable here for cross-SDK consistency.
13-17: LGTM!The
Stringextension ensures backward compatibility for existing callers passing string channels directly.
154-163: LGTM!The signature change to
ChannelValueis backward compatible, and the delegation logic is correct.
165-174: Breaking API change:Set<String>→[ChannelValue].Changing the parameter type from
Set<String>to[ChannelValue]is a breaking change for callers using theSettype. While migration is straightforward, this is worth noting in release documentation.The implementation correctly converts to
Set<String>internally, preserving deduplication behavior.
176-186: LGTM!The generic overload correctly handles the
ChannelValueparameter and delegates appropriately.
188-198: LGTM!This new overload completes the API surface, allowing multi-channel subscriptions with typed payloads using
ChannelValue. Implementation is consistent with other overloads.
19-31: No action required. All Channel types (ResolvedChannel,DatabaseChannel, etc.) already implement thetoString() -> Stringmethod in their base definitions, satisfying theChannelValueprotocol requirements. The empty extensions correctly declare protocol conformance.templates/apple/Sources/Channel.swift.twig (1)
1-261: LGTM! Well-structured Channel DSL for Swift.The implementation provides a clean, fluent API for constructing channel strings with proper type safety and default wildcard support. The class hierarchy (ResolvedChannel, ActionChannel, resource-specific channels) is well-organized and follows Swift conventions. Based on learnings, this intentionally follows the SDK pattern of accepting IDs as-is without validation or escaping.
tests/languages/flutter/tests.dart (1)
250-270: Excellent test coverage for Channel helpers.The Channel helper tests comprehensively exercise all factory methods (database, tablesdb, account, buckets, functions, teams, memberships) with various scenarios: default wildcards, specific IDs, nested builders, and action methods. Test patterns are consistent with other helper tests in the file.
tests/languages/web/index.html (2)
24-24: Import updated correctly.Channel properly added to the public API imports for testing.
322-342: Comprehensive Channel helper test coverage.Tests thoroughly exercise all Channel factory methods and builders with appropriate scenarios including default wildcards, specific IDs, nested chains, and action methods (create/update/delete). Coverage aligns with other language test implementations.
tests/languages/web/node.js (2)
1-1: Import correctly updated for Node.js.Channel added to CommonJS require imports.
253-273: Complete Channel helper test coverage.Node.js tests mirror the browser test coverage, ensuring Channel helpers work correctly in both environments. All factory methods and builder patterns are validated.
tests/languages/android/Tests.kt (2)
10-10: Import added correctly.Channel class imported for test usage.
273-293: Complete Channel helper test coverage.The 20 Channel helper test cases comprehensively cover all factory methods (database, tablesdb, account, buckets, functions, teams, memberships) with default wildcards, specific IDs, and actions. Coverage is consistent with other language implementations.
templates/web/src/client.ts.twig (1)
2-2: Import added correctly.ChannelValue imported for type support in subscribe method.
templates/android/library/src/main/java/io/package/Channel.kt.twig (1)
1-190: Excellent Kotlin Channel DSL implementation.The implementation provides a clean, idiomatic Kotlin API with proper use of default parameters, companion object for factory methods, and a well-organized class hierarchy. The design is consistent with other language implementations while following Kotlin best practices. Based on learnings, this correctly follows the SDK pattern of accepting IDs without validation or escaping.
templates/web/src/services/realtime.ts.twig (3)
2-2: Import added correctly.ChannelValue imported with proper relative path from service subdirectory.
241-257: Clean channel normalization helper.The channelToString helper method provides a clean, reusable way to normalize both string and ChannelValue inputs. The logic handles all cases appropriately with proper type checking and fallback.
259-348: Well-designed subscribe overloads with ChannelValue support.The implementation provides excellent type safety through multiple overloads, allowing users to pass single or multiple channels as either strings or Channel builder instances. The normalization logic (lines 311-317) cleanly converts all inputs to a Set for internal handling. Documentation is comprehensive and the use of the channelToString helper keeps the code maintainable.
ArnabChatterjee20k
force-pushed
the
dat-971
branch
from
93d0b02 to
3e2402d
Compare
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
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.