Added channel helpers for realtime in the client sdks #1266
Conversation
WalkthroughAdds Channel builder templates and generated file mappings across Android (Kotlin), Apple (Swift), Flutter (Dart), React Native (TS), and Web (TS). Introduces ResolvedChannel, Actionable, and fluent Channel builders in language templates; exposes Channel in index files; updates Realtime and client APIs to accept Channel-like values (Actionable/ResolvedChannel/Object) alongside strings; adjusts Swift WebSocket types to use NIOCore.Channel; and extends tests and test expectations to include channel-related cases. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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.
templates/web/src/channel.ts.twig
Outdated
| 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
| part of '{{ language.params.packageName }}.dart'; | ||
|
|
||
| class ResolvedChannel { | ||
| final String _value; | ||
|
|
||
| ResolvedChannel(this._value); | ||
|
|
||
| String toString() { | ||
| return _value; | ||
| } | ||
| } | ||
|
|
||
| abstract class ActionChannel { | ||
| final String base; | ||
|
|
||
| ActionChannel(this.base); | ||
|
|
||
| ResolvedChannel create() { | ||
| return ResolvedChannel('$base.create'); | ||
| } | ||
|
|
||
| ResolvedChannel update() { | ||
| return ResolvedChannel('$base.update'); | ||
| } | ||
|
|
||
| ResolvedChannel delete() { | ||
| return ResolvedChannel('$base.delete'); | ||
| } | ||
|
|
||
| @override | ||
| String toString() { | ||
| return base; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * ---------------------------- | ||
| * Database → Collection → Document | ||
| * ---------------------------- | ||
| */ | ||
| class DocumentChannel extends ActionChannel { | ||
| DocumentChannel(String base) : super(base); | ||
| } | ||
|
|
||
| class CollectionChannel { | ||
| final String base; | ||
| final String databaseId; | ||
| final String collectionId; | ||
|
|
||
| CollectionChannel(this.databaseId, this.collectionId) | ||
| : base = 'databases.$databaseId.collections.$collectionId'; | ||
|
|
||
| DocumentChannel document([String documentId = '*']) { | ||
| return DocumentChannel('$base.documents.$documentId'); | ||
| } | ||
|
|
||
| @override | ||
| String toString() { | ||
| return base; | ||
| } | ||
| } | ||
|
|
||
| class DatabaseChannel { | ||
| final String base; | ||
| final String databaseId; | ||
|
|
||
| DatabaseChannel(this.databaseId) : base = 'databases.$databaseId'; | ||
|
|
||
| CollectionChannel collection([String collectionId = '*']) { | ||
| return CollectionChannel(databaseId, collectionId); | ||
| } | ||
|
|
||
| @override | ||
| String toString() { | ||
| return base; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * ---------------------------- | ||
| * TablesDB → Table → Row | ||
| * ---------------------------- | ||
| */ | ||
| class RowChannel extends ActionChannel { | ||
| RowChannel(String base) : super(base); | ||
| } | ||
|
|
||
| class TableChannel { | ||
| final String base; | ||
| final String databaseId; | ||
| final String tableId; | ||
|
|
||
| TableChannel(this.databaseId, this.tableId) | ||
| : base = 'tablesdb.$databaseId.tables.$tableId'; | ||
|
|
||
| RowChannel row([String rowId = '*']) { | ||
| return RowChannel('$base.rows.$rowId'); | ||
| } | ||
|
|
||
| @override | ||
| String toString() { | ||
| return base; | ||
| } | ||
| } | ||
|
|
||
| class TablesDBChannel { | ||
| final String base; | ||
| final String databaseId; | ||
|
|
||
| TablesDBChannel(this.databaseId) : base = 'tablesdb.$databaseId'; | ||
|
|
||
| TableChannel table([String tableId = '*']) { | ||
| return TableChannel(databaseId, tableId); | ||
| } | ||
|
|
||
| @override | ||
| String toString() { | ||
| return base; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * ---------------------------- | ||
| * Buckets → File | ||
| * ---------------------------- | ||
| */ | ||
| class FileChannel extends ActionChannel { | ||
| FileChannel(String base) : super(base); | ||
| } | ||
|
|
||
| class BucketChannel { | ||
| final String base; | ||
| final String bucketId; | ||
|
|
||
| BucketChannel(this.bucketId) : base = 'buckets.$bucketId'; | ||
|
|
||
| FileChannel file([String fileId = '*']) { | ||
| return FileChannel('$base.files.$fileId'); | ||
| } | ||
|
|
||
| @override | ||
| String toString() { | ||
| return base; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * ---------------------------- | ||
| * Functions → Execution | ||
| * ---------------------------- | ||
| */ | ||
| class ExecutionChannel extends ActionChannel { | ||
| ExecutionChannel(String base) : super(base); | ||
| } | ||
|
|
||
| class FunctionChannel { | ||
| final String base; | ||
| final String functionId; | ||
|
|
||
| FunctionChannel(this.functionId) : base = 'functions.$functionId'; | ||
|
|
||
| ExecutionChannel execution([String executionId = '*']) { | ||
| return ExecutionChannel('$base.executions.$executionId'); | ||
| } | ||
|
|
||
| @override | ||
| String toString() { | ||
| return base; | ||
| } | ||
| } | ||
|
|
||
| class TeamChannel extends ActionChannel { | ||
| TeamChannel([String teamId = '*']) : super('teams.$teamId'); | ||
| } | ||
|
|
||
| class MembershipChannel extends ActionChannel { | ||
| MembershipChannel([String membershipId = '*']) | ||
| : super('memberships.$membershipId'); | ||
| } | ||
|
|
||
| typedef ChannelValue = Object; | ||
|
|
||
| class Channel { | ||
| /// Generate a database channel builder. | ||
| /// | ||
| /// [databaseId] The database ID (default: "*") | ||
| /// Returns [DatabaseChannel] | ||
| static DatabaseChannel database([String databaseId = '*']) { | ||
| return DatabaseChannel(databaseId); | ||
| } | ||
|
|
||
| /// Generate a tables database channel builder. | ||
| /// | ||
| /// [databaseId] The database ID (default: "*") | ||
| /// Returns [TablesDBChannel] | ||
| static TablesDBChannel tablesdb([String databaseId = '*']) { | ||
| return TablesDBChannel(databaseId); | ||
| } | ||
|
|
||
| /// Generate a buckets channel builder. | ||
| /// | ||
| /// [bucketId] The bucket ID (default: "*") | ||
| /// Returns [BucketChannel] | ||
| static BucketChannel buckets([String bucketId = '*']) { | ||
| return BucketChannel(bucketId); | ||
| } | ||
|
|
||
| /// Generate a functions channel builder. | ||
| /// | ||
| /// [functionId] The function ID (default: "*") | ||
| /// Returns [FunctionChannel] | ||
| static FunctionChannel functions([String functionId = '*']) { | ||
| return FunctionChannel(functionId); | ||
| } | ||
|
|
||
| /// Generate a teams channel builder. | ||
| /// | ||
| /// [teamId] The team ID (default: "*") | ||
| /// Returns [TeamChannel] | ||
| static TeamChannel teams([String teamId = '*']) { | ||
| return TeamChannel(teamId); | ||
| } | ||
|
|
||
| /// Generate a memberships channel builder. | ||
| /// | ||
| /// [membershipId] The membership ID (default: "*") | ||
| /// Returns [MembershipChannel] | ||
| static MembershipChannel memberships([String membershipId = '*']) { | ||
| return MembershipChannel(membershipId); | ||
| } | ||
|
|
||
| /// Generate an account channel string. | ||
| /// | ||
| /// [userId] The user ID (default: "*") | ||
| /// Returns The channel string | ||
| static String account([String userId = '*']) { | ||
| return 'account.$userId'; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
It was interesting to look at the PR, and I noticed signs of Class Explosion and a violation of the DRY principle.
I propose a version with a single Builder and a system of interface constraints. It preserves strict typing of calls, ridding the project of unnecessary classes and duplication of logic.
part of appwrite_channel;
// The result of building the channel.
final class ResolvedChannel {
final String value;
const ResolvedChannel(this.value);
@override
String toString() => value;
}
// Helper function for normalizing ID
String _normalize(String id) => id.trim().isEmpty ? '*' : id;
// State interfaces
abstract interface class Actionable {
ResolvedChannel create();
ResolvedChannel update();
ResolvedChannel delete();
String get channel;
}
abstract interface class IDatabase {
ICollection collection([String id = '*']);
}
abstract interface class ICollection {
IDocument document([String id = '*']);
}
abstract interface class IDocument implements Actionable {}
abstract interface class ITablesDB {
ITable table([String id = '*']);
}
abstract interface class ITable {
IRow row([String id = '*']);
}
abstract interface class IRow implements Actionable {}
abstract interface class IBucket {
IFile file([String id = '*']);
}
abstract interface class IFile implements Actionable {}
abstract interface class IFunction {
IExecution execution([String id = '*']);
}
abstract interface class IExecution implements Actionable {}
// Builder implementation
class _ChannelBuilder implements
IDatabase, ICollection, IDocument,
ITablesDB, ITable, IRow,
IBucket, IFile,
IFunction, IExecution, Actionable {
final List<String> _segments;
_ChannelBuilder._(this._segments);
_ChannelBuilder _next(String segment, String id) {
return _ChannelBuilder._([
..._segments,
segment,
_normalize(id),
]);
}
@override
ICollection collection([String id = '*']) => _next('collections', id);
@override
IDocument document([String id = '*']) => _next('documents', id);
@override
ITable table([String id = '*']) => _next('tables', id);
@override
IRow row([String id = '*']) => _next('rows', id);
@override
IFile file([String id = '*']) => _next('files', id);
@override
IExecution execution([String id = '*']) => _next('executions', id);
@override
ResolvedChannel create() => _resolve('create');
@override
ResolvedChannel update() => _resolve('update');
@override
ResolvedChannel delete() => _resolve('delete');
@override
String get channel => _segments.join('.');
@override
String toString() => channel;
ResolvedChannel _resolve(String action) => ResolvedChannel('$channel.$action');
}
// Entry point
abstract final class Channel {
static IDatabase database([String id = '*']) =>
_ChannelBuilder._(['databases', _normalize(id)]);
static ITablesDB tablesdb([String id = '*']) =>
_ChannelBuilder._(['tablesdb', _normalize(id)]);
static IBucket buckets([String id = '*']) =>
_ChannelBuilder._(['buckets', _normalize(id)]);
static IFunction functions([String id = '*']) =>
_ChannelBuilder._(['functions', _normalize(id)]);
static Actionable teams([String id = '*']) =>
_ChannelBuilder._(['teams', _normalize(id)]);
static Actionable memberships([String id = '*']) =>
_ChannelBuilder._(['memberships', _normalize(id)]);
static String account([String userId = '']) =>
userId.isEmpty ? 'account' : 'account.${_normalize(userId)}';
// Global events (without ID)
static String get documents => 'documents';
static String get rows => 'rows';
static String get files => 'files';
static String get executions => 'executions';
}There was a problem hiding this comment.
Thanks for the suggestion. It really makes sense. Will surely apply this
There was a problem hiding this comment.
Glad it helped,
I also looked at the classes, and there is no single builder used for Kotlin and Swift.
There was a problem hiding this comment.
Glad it helped, I also looked at the classes, and there is no single builder used for Kotlin and Swift.
I just pushed swift and kotlin channel builder. Should be good now I think, waiting for the tests
…lates - Removed deprecated channel types and streamlined channel value handling in Realtime service. - Introduced Actionable and ResolvedChannel interfaces for better type safety. - Updated database, tablesdb, buckets, functions, teams, and memberships channel builders to use new interfaces. - Enhanced test cases to reflect changes in channel creation and subscription methods. - Improved documentation for channel subscription methods to clarify expected types.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
templates/apple/Sources/Services/Realtime.swift.twig (1)
136-153: Consider simplifying the channel conversion logic.The explicit check for
ResolvedChannelat lines 148-150 appears redundant. SinceResolvedChannelconforms toChannelValue, callingchannel.toString()in the fallback (line 152) would produce the same result.The code could be simplified to:
private func channelToString(_ channel: ChannelValue) -> String { // Check if it's an Actionable instance (has channel property) if let actionable = channel as? Actionable { return actionable.channel } // Use toString() for all ChannelValue types return channel.toString() }This eliminates the redundant branch while maintaining the special handling for
Actionabletypes.templates/flutter/lib/src/realtime_mixin.dart.twig (1)
171-185: Redundant type check and cast.The check
if (channel is ResolvedChannel)on line 181 is redundant because line 184 already handles this case viachannel.toString(). All Dart objects have atoString()method, so ResolvedChannel will be handled correctly by the fallback.Additionally, the
.cast<String>()on line 189 is unnecessary sincemap((ch) => _channelToString(ch))already returnsIterable<String>, andtoList()preserves that type.🔎 Suggested simplification
/// Convert channel value to string String _channelToString(Object channel) { if (channel is String) { return channel; } // Check if it's an Actionable instance (has channel getter) if (channel is Actionable) { return channel.channel; } - // Check if it's a ResolvedChannel (has toString() method) - if (channel is ResolvedChannel) { - return channel.toString(); - } return channel.toString(); } RealtimeSubscription subscribeTo(List<Object> channels) { StreamController<RealtimeMessage> controller = StreamController.broadcast(); - final channelStrings = channels.map((ch) => _channelToString(ch)).toList().cast<String>(); + final channelStrings = channels.map((ch) => _channelToString(ch)).toList();templates/web/src/client.ts.twig (1)
585-601: Redundant type check fortoString.The check on line 597
typeof (ch as ResolvedChannel).toString === 'function'is always true since all JavaScript objects inherittoStringfromObject.prototype. This branch will handle all non-string, non-Actionable inputs, making theString(ch)fallback on line 600 unreachable.Consider simplifying by removing the redundant check:
🔎 Suggested simplification
const channelStrings = channelArray.map(ch => { if (typeof ch === 'string') { return ch; } // Check if it's an Actionable instance (has channel() method) if (ch && typeof (ch as Actionable).channel === 'function') { return (ch as Actionable).channel(); } - // Check if it's a ResolvedChannel (has toString() method) - if (ch && typeof (ch as ResolvedChannel).toString === 'function') { - return (ch as ResolvedChannel).toString(); - } - return String(ch); + return ch.toString(); });templates/web/src/services/realtime.ts.twig (1)
248-261: Same redundanttoStringcheck as in client.ts.Lines 256-259 check for
toStringmethod existence, but all objects have this method. The check will always pass, making line 260'sString(channel)unreachable.🔎 Suggested simplification
private channelToString(channel: string | Actionable | ResolvedChannel): string { if (typeof channel === 'string') { return channel; } // Check if it's an Actionable instance (has channel() method) if (channel && typeof (channel as Actionable).channel === 'function') { return (channel as Actionable).channel(); } - // Check if it's a ResolvedChannel (has toString() method) - if (channel && typeof (channel as ResolvedChannel).toString === 'function') { - return (channel as ResolvedChannel).toString(); - } - return String(channel); + return channel.toString(); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
templates/android/library/src/main/java/io/package/Channel.kt.twigtemplates/android/library/src/main/java/io/package/services/Realtime.kt.twigtemplates/apple/Sources/Channel.swift.twigtemplates/apple/Sources/Services/Realtime.swift.twigtemplates/dart/test/channel_test.dart.twigtemplates/flutter/lib/channel.dart.twigtemplates/flutter/lib/src/realtime.dart.twigtemplates/flutter/lib/src/realtime_base.dart.twigtemplates/flutter/lib/src/realtime_mixin.dart.twigtemplates/react-native/src/channel.ts.twigtemplates/web/src/channel.ts.twigtemplates/web/src/client.ts.twigtemplates/web/src/services/realtime.ts.twig
🚧 Files skipped from review as they are similar to previous changes (2)
- templates/dart/test/channel_test.dart.twig
- templates/android/library/src/main/java/io/package/services/Realtime.kt.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:35.419Z
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:20.227Z
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:20.227Z
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/swift/Sources/WebSockets/WebSocketClient.swift.twig:140-140
Timestamp: 2025-12-18T18:42:20.227Z
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_mixin.dart.twigtemplates/flutter/lib/src/realtime_base.dart.twigtemplates/web/src/client.ts.twigtemplates/flutter/lib/src/realtime.dart.twigtemplates/android/library/src/main/java/io/package/Channel.kt.twigtemplates/apple/Sources/Channel.swift.twigtemplates/flutter/lib/channel.dart.twigtemplates/react-native/src/channel.ts.twigtemplates/apple/Sources/Services/Realtime.swift.twigtemplates/web/src/services/realtime.ts.twigtemplates/web/src/channel.ts.twig
📚 Learning: 2025-12-18T18:43:35.419Z
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/apple/Sources/Channel.swift.twig:13-19
Timestamp: 2025-12-18T18:43:35.419Z
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:
templates/android/library/src/main/java/io/package/Channel.kt.twigtemplates/apple/Sources/Channel.swift.twigtemplates/flutter/lib/channel.dart.twigtemplates/react-native/src/channel.ts.twigtemplates/web/src/channel.ts.twig
🔇 Additional comments (18)
templates/apple/Sources/Services/Realtime.swift.twig (3)
6-17: LGTM! Clean abstraction for channel types.The ChannelValue protocol provides a simple, extensible contract for channel representation. String conformance ensures backward compatibility, allowing existing code using String channels to work seamlessly with the new API.
19-20: ResolvedChannel and ActionChannel correctly implement the required ChannelValue protocol.Both types already define
toString() -> Stringmethods in Channel.swift.twig (ResolvedChannel at lines 11-13, ActionChannel at lines 54-56), making the empty protocol conformance extensions valid.
155-199: LGTM! Well-structured API expansion with backward compatibility.The subscribe overloads correctly accept
ChannelValuetypes while maintaining internalSet<String>logic. Key strengths:
- Backward compatible: Existing String-based calls continue to work seamlessly
- Consistent pattern: All overloads convert ChannelValue → String via
channelToStringbefore forwarding- Complete API surface: New
channels: [ChannelValue], payloadType:overload (lines 189-199) fills a gap in the generic API- Properly typed:
Actionableprotocol is correctly defined in Channel.swift.twig with the.channelproperty, ensuring type-safe conversionsThe clean separation between the public ChannelValue-based interface and internal String-based storage ensures no breaking changes.
templates/flutter/lib/src/realtime.dart.twig (1)
45-53: LGTM!The updated documentation with Channel builder examples is clear and helpful. The API change from
List<String>toList<Object>maintains backward compatibility since strings are still accepted, while enabling the new Channel builder pattern.templates/flutter/lib/src/realtime_base.dart.twig (1)
4-6: LGTM!The signature change is consistent with the parent
Realtimeclass interface.templates/web/src/client.ts.twig (1)
575-582: Good documentation addition.The Channel builder examples in the JSDoc are clear and demonstrate the various builder patterns available.
templates/android/library/src/main/java/io/package/Channel.kt.twig (1)
1-196: Well-structured Channel DSL for Android/Kotlin.The implementation provides good type safety with separate classes for each hierarchy level. The
ActionChannelbase class effectively reduces duplication for action methods. Thenormalizehelper correctly handles empty/whitespace IDs by defaulting to"*".Note: The Flutter implementation (in
channel.dart.twig) uses a single_ChannelBuilderclass with interfaces, which is more compact. Both approaches are valid - this one trades more classes for clearer static typing at each builder step.templates/flutter/lib/channel.dart.twig (2)
131-132: Consider edge case for whitespace-only userId.The
accountmethod checksuserId.isEmptybut if a user passes whitespace like" ", it won't be considered empty and will result inaccount.*(after normalization). This differs from other entry points where the ID is normalized first.Is this the intended behavior? If consistency is desired:
🔎 Potential fix for consistency
static String account([String userId = '']) => - userId.isEmpty ? 'account' : 'account.${_normalize(userId)}'; + userId.trim().isEmpty ? 'account' : 'account.${_normalize(userId)}';
58-108: Well-implemented consolidated builder pattern.The
_ChannelBuilderclass implementing all interfaces via a segment list is clean and follows DRY principles. This addresses the "class explosion" concern raised in the past review discussion.templates/web/src/services/realtime.ts.twig (1)
311-321: LGTM!The subscribe implementation correctly normalizes mixed channel types to a
Set<string>before registering with the subscription system.templates/react-native/src/channel.ts.twig (1)
1-195: LGTM - consistent Channel builder implementation.The React Native implementation follows the same consolidated builder pattern as Flutter, with a single
ChannelBuilderclass implementing all level interfaces. Theaccount()method correctly handles whitespace withtrim()check (line 173), which is consistent with how other platforms should behave.Minor note: The file has unusual indentation (extra leading spaces on most lines), which may be a template formatting artifact.
templates/web/src/channel.ts.twig (3)
1-13: LGTM! Clean foundation utilities.The
ResolvedChannelwrapper andnormalizehelper are well-implemented. The immutable design ofResolvedChanneland the whitespace-to-wildcard normalization logic are both sound.
17-54: LGTM! Well-structured type-safe interfaces.The interface hierarchy correctly models the resource relationships and ensures type-safe chaining. The separation of concerns between chainable interfaces and the
Actionableinterface for terminal operations is clean.
58-143: LGTM! Solid builder implementation.The immutable builder pattern with
next()creating new instances is a good design choice. The approach of havingChannelBuilderimplement all interfaces works well in TypeScript's structural type system, keeping the implementation DRY while the interface types enforce correct usage at the API boundary.templates/apple/Sources/Channel.swift.twig (4)
1-20: LGTM! Solid foundation matching TypeScript implementation.The
ResolvedChannelandnormalizeutilities correctly mirror the TypeScript implementation. The immutability ofResolvedChanneland the whitespace normalization logic are both sound.Based on learnings, the lack of ID validation is intentional design for SDK helper utilities.
23-57: LGTM! Appropriate Swift design pattern.The
Actionableprotocol withActionChannelbase class is a clean Swift pattern that provides code reuse while maintaining type safety. This is a good language-specific adaptation compared to the TypeScript interface-based approach.
59-199: LGTM! Consistent and well-structured channel hierarchy.All resource-specific channel classes follow a consistent pattern:
- Intermediate builders (CollectionChannel, TableChannel, BucketChannel, FunctionChannel) provide chaining methods
- Terminal channels (DocumentChannel, RowChannel, FileChannel, ExecutionChannel, TeamChannel, MembershipChannel) extend
ActionChannelfor CRUD actions- Each class properly normalizes IDs and constructs correct dot-delimited channel paths
The verbose class-per-resource approach is appropriate for Swift's type system and provides excellent type safety. The channel strings generated match the TypeScript implementation (except for the account method, flagged separately).
236-239: LGTM! Global event constants match TypeScript implementation.The static constants for global events (
documents,rows,files,executions) are consistent with the TypeScript implementation and provide a clean API for subscribing to global resource events.
| public static func account(_ userId: String = "") -> String { | ||
| let normalized = normalize(userId) | ||
| return normalized == "*" ? "account" : "account.\(normalized)" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine the Swift Channel implementation
find . -path "*templates/apple/Sources/Channel.swift.twig" -type fRepository: appwrite/sdk-generator
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Find and examine the TypeScript Channel implementation
find . -path "*templates/web/src/channel.ts.twig" -type fRepository: appwrite/sdk-generator
Length of output: 101
🏁 Script executed:
#!/bin/bash
# List relevant template files to understand structure
git ls-files | grep -E "(Channel|channel)" | grep -E "\.(swift|ts)\.twig"Repository: appwrite/sdk-generator
Length of output: 185
🏁 Script executed:
#!/bin/bash
# Read the Swift Channel template file, focusing on lines 229-232 and surrounding context
sed -n '220,240p' ./templates/apple/Sources/Channel.swift.twigRepository: appwrite/sdk-generator
Length of output: 707
🏁 Script executed:
#!/bin/bash
# Read the TypeScript Channel template file, focusing on lines 172-176 and surrounding context
sed -n '165,185p' ./templates/web/src/channel.ts.twigRepository: appwrite/sdk-generator
Length of output: 563
🏁 Script executed:
#!/bin/bash
# Also get a broader view of the TypeScript file to understand the account method better
sed -n '160,190p' ./templates/web/src/channel.ts.twigRepository: appwrite/sdk-generator
Length of output: 808
🏁 Script executed:
#!/bin/bash
# Check the normalize function in both files
echo "=== Swift normalize function ==="
grep -n "normalize" ./templates/apple/Sources/Channel.swift.twig | head -10
echo ""
echo "=== TypeScript normalize function ==="
grep -n "normalize" ./templates/web/src/channel.ts.twig | head -10Repository: appwrite/sdk-generator
Length of output: 1214
🏁 Script executed:
#!/bin/bash
# Get the full context of normalize in Swift
sed -n '1,50p' ./templates/apple/Sources/Channel.swift.twigRepository: appwrite/sdk-generator
Length of output: 1177
🏁 Script executed:
#!/bin/bash
# Get the full context of normalize in TypeScript
sed -n '1,50p' ./templates/web/src/channel.ts.twigRepository: appwrite/sdk-generator
Length of output: 1054
Account method behavior inconsistent with TypeScript implementation.
Swift's account() method returns "account" when the normalized input equals "*", while the TypeScript version (templates/web/src/channel.ts.twig lines 171-175) returns "account.*" for the same input.
Both normalize() functions are equivalent, but the decision logic differs: Swift checks if normalized == "*" before formatting, while TypeScript checks if the input is non-empty and always applies the format suffix. This creates a cross-platform API inconsistency where Channel.account("*") returns different values.
Coordinate with the Web SDK to align behavior.
🤖 Prompt for AI Agents
In templates/apple/Sources/Channel.swift.twig around lines 229 to 232, the
account(_:) method special-cases normalized == "*" and returns "account", which
diverges from the TypeScript implementation that returns "account.*" for "*"
inputs; update the decision logic to mirror the Web SDK by returning "account"
only when normalized is empty and otherwise returning "account.\(normalized)" so
"*" becomes "account.*" and behavior is consistent across platforms.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
templates/flutter/lib/channel.dart.twig (1)
112-138: Clean public API with appropriate type guidance.The
Channelclass provides intuitive static entry points that return typed interfaces guiding users through valid builder chains. The distinction between structured resources (database, buckets, etc.) returning builders and simpler resources (account, global events) returning strings directly makes sense given their different structures.Optional: More idiomatic Dart pattern
While the abstract class with static members works correctly, a more idiomatic Dart pattern would use a private constructor to prevent instantiation:
-abstract class Channel { +class Channel { + // Private constructor prevents instantiation + Channel._(); + static IDatabase database([String id = '*']) => _ChannelBuilder._(['databases', _normalize(id)]); // ... rest of static methodsThis is purely stylistic and doesn't affect functionality.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
templates/flutter/lib/channel.dart.twigtemplates/flutter/lib/src/realtime_mixin.dart.twigtests/Base.php
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/flutter/lib/src/realtime_mixin.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:35.419Z
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:20.227Z
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:20.227Z
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/swift/Sources/WebSockets/WebSocketClient.swift.twig:140-140
Timestamp: 2025-12-18T18:42:20.227Z
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/channel.dart.twig
📚 Learning: 2025-12-18T18:43:35.419Z
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/apple/Sources/Channel.swift.twig:13-19
Timestamp: 2025-12-18T18:43:35.419Z
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:
templates/flutter/lib/channel.dart.twig
⏰ 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: swift (server)
- GitHub Check: dart (server)
- GitHub Check: kotlin (server)
- GitHub Check: react-native (client)
- GitHub Check: php (server)
- GitHub Check: node (server)
- GitHub Check: android (client)
- GitHub Check: build (8.3, CLINode18)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, CLINode20)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, DotNet60)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, Ruby30)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, Android14Java17)
🔇 Additional comments (4)
tests/Base.php (1)
166-187: LGTM! Comprehensive test data for channel helpers.The
CHANNEL_HELPER_RESPONSESconstant provides thorough coverage of channel patterns across different resource types (databases, tablesdb, account, buckets, functions, teams, memberships) with wildcards, specific IDs, and action variants. The patterns correctly follow the dot-notation format expected by Appwrite realtime channels.templates/flutter/lib/channel.dart.twig (3)
3-13: Well-designed value types and normalization.The
ResolvedChannelwrapper provides semantic distinction for channels with actions, and the_normalizehelper correctly implements the minimal ID transformation pattern (empty → '*'). Based on learnings, the lack of ID validation or escaping is intentional across SDK helpers.
17-54: Excellent use of interfaces for type-safe fluent API.The interface hierarchy enforces valid call sequences at compile time while avoiding class explosion. Each interface exposes only the appropriate next steps in the builder chain, with terminal types implementing
Actionableto provide action methods and channel access. This directly addresses the previous review feedback.
58-108: Solid single-builder implementation addressing previous feedback.The
_ChannelBuilderclass elegantly implements all interfaces while maintaining type safety through return types. The immutable segment list and_next()pattern prevent accidental mutations. This design directly implements the suggestion from the previous review to use "a single Builder and a system of interface constraints" rather than multiple classes, successfully avoiding class explosion while preserving strict typing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
templates/flutter/lib/src/realtime_mixin.dart.twig (1)
194-194: Remove redundant cast operation.The
.cast<String>()is unnecessary sincemap(_channelToString)already producesList<String>after.toList().🔎 Simplified implementation
- final channelStrings = channels.map((ch) => _channelToString(ch)).toList().cast<String>(); + final channelStrings = channels.map((ch) => _channelToString(ch)).toList();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/flutter/lib/src/realtime_mixin.dart.twig
🧰 Additional context used
🧠 Learnings (2)
📓 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:35.419Z
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:20.227Z
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:20.227Z
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/swift/Sources/WebSockets/WebSocketClient.swift.twig:140-140
Timestamp: 2025-12-18T18:42:20.227Z
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_mixin.dart.twig
⏰ 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, WebChromium)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: build (8.3, Node16)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, Python39)
- GitHub Check: build (8.3, Node20)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: flutter (client)
- GitHub Check: kotlin (server)
- GitHub Check: android (client)
- GitHub Check: swift (server)
- GitHub Check: apple (client)
🔇 Additional comments (3)
templates/flutter/lib/src/realtime_mixin.dart.twig (3)
171-190: LGTM! Dynamic channel conversion implementation is appropriate.The three-tier approach (String → Actionable interface → toString fallback) correctly handles multiple channel representations. The dynamic property access avoids import conflicts with part file types, which is a pragmatic solution given Dart's part/library constraints.
192-192: LGTM! Backward-compatible signature change.Broadening the parameter type from
List<String>toList<Object>enables Channel helper objects while maintaining backward compatibility with existing string-based callers.
195-204: LGTM! Consistent use of normalized channel strings.The normalized
channelStringsare used consistently throughout the method, ensuring type safety for internal storage, subscription creation, and cleanup operations.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
templates/apple/Sources/Channel.swift.twig (1)
151-154: Cross-platform API inconsistency with TypeScript implementation remains.This was flagged in a previous review: when
userIdis empty or whitespace (normalized to"*"), Swift returns"account"while TypeScript returns"account.*". Please coordinate with the Web SDK to align behavior.
🧹 Nitpick comments (1)
templates/apple/Sources/Channel.swift.twig (1)
4-14: Consider conforming toCustomStringConvertiblefor Swift idiom.The
toString()method works, but Swift convention prefersCustomStringConvertibleconformance with adescriptionproperty. This allowsResolvedChannelto work naturally with string interpolation and print statements.🔎 Suggested enhancement (optional)
-public final class ResolvedChannel { +public final class ResolvedChannel: CustomStringConvertible { private let value: String init(value: String) { self.value = value } public func toString() -> String { value } + + public var description: String { + value + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
templates/android/library/src/main/java/io/package/Channel.kt.twigtemplates/apple/Sources/Channel.swift.twig
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/android/library/src/main/java/io/package/Channel.kt.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:35.419Z
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:20.227Z
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:20.227Z
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/swift/Sources/WebSockets/WebSocketClient.swift.twig:140-140
Timestamp: 2025-12-18T18:42:20.227Z
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/apple/Sources/Channel.swift.twig
📚 Learning: 2025-12-18T18:43:35.419Z
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/apple/Sources/Channel.swift.twig:13-19
Timestamp: 2025-12-18T18:43:35.419Z
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:
templates/apple/Sources/Channel.swift.twig
⏰ 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, Go118)
- GitHub Check: build (8.3, Python311)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, Ruby30)
- GitHub Check: build (8.3, Python39)
- GitHub Check: build (8.3, Python312)
- GitHub Check: build (8.3, PHP83)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, Node16)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, Python313)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: apple (client)
- GitHub Check: swift (server)
- GitHub Check: android (client)
🔇 Additional comments (3)
templates/apple/Sources/Channel.swift.twig (3)
16-20: LGTM!The normalize function correctly handles trimming and defaults empty/whitespace-only inputs to
"*".
22-60: LGTM!The protocol hierarchy is well-designed, enabling type-safe fluent chaining for channel construction. The separation between navigation protocols (IDatabase, ICollection, etc.) and terminal actionable protocols (IDocument, IRow, IFile, IExecution) is clean.
125-149: LGTM!The public API is well-designed with appropriate return types that hide the internal
ChannelBuilderimplementation. The static factory methods provide a clean entry point for channel construction, and the global event constants are appropriately defined.Also applies to: 156-161
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
templates/apple/Sources/Channel.swift.twig (1)
208-211: Cross-platform inconsistency: account() behavior still differs from TypeScript.The
account()method continues to return"account"when the normalized input is"*", while the TypeScript implementation returns"account.*". This creates inconsistent channel strings across SDKs:
- Swift:
Channel.account("*")→"account"- TypeScript:
Channel.account("*")→"account.*"This was flagged as critical in a previous review and remains unresolved.
Based on the previous review thread, the expected fix is to return
"account.*"when the normalized ID is"*", aligning with the Web SDK behavior.
🧹 Nitpick comments (2)
tests/AppleSwift56Test.php (2)
138-154: Consider adding error handling for filesystem operations.The
rmdirRecursive()implementation is correct, but filesystem operations likeunlink(),rmdir(), andscandir()can fail due to permission issues or other errors. Consider wrapping these operations in a try-catch block and logging or throwing exceptions on failure to make debugging easier if cleanup fails.🔎 Proposed enhancement with error handling
private function rmdirRecursive($dir): void { if (!\is_dir($dir)) { return; } - foreach (\scandir($dir) as $file) { - if ('.' === $file || '..' === $file) { - continue; - } - if (\is_dir("$dir/$file")) { - $this->rmdirRecursive("$dir/$file"); - } else { - \unlink("$dir/$file"); + try { + foreach (\scandir($dir) as $file) { + if ('.' === $file || '..' === $file) { + continue; + } + if (\is_dir("$dir/$file")) { + $this->rmdirRecursive("$dir/$file"); + } else { + \unlink("$dir/$file"); + } } + \rmdir($dir); + } catch (\Exception $e) { + echo "Warning: Failed to remove directory $dir: " . $e->getMessage() . "\n"; } - \rmdir($dir); }
38-136: Consider refactoring this method to improve maintainability.The
testHTTPSuccess()method handles multiple distinct responsibilities sequentially:
- SDK setup and configuration (lines 43-69)
- Directory cleanup and generation (lines 71-75)
- Building (lines 77-83)
- Test execution and output capture (lines 85-97)
- Output validation (lines 99-135)
While the enhanced logging for debugging is valuable, extracting helper methods like
setupSdk(),buildSdk(),captureTestOutput(), andvalidateTestOutput()would improve readability and testability.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
templates/apple/Sources/Channel.swift.twigtemplates/apple/Sources/Services/Realtime.swift.twigtests/AppleSwift56Test.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:35.419Z
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:20.227Z
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:20.227Z
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/swift/Sources/WebSockets/WebSocketClient.swift.twig:140-140
Timestamp: 2025-12-18T18:42:20.227Z
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/apple/Sources/Services/Realtime.swift.twigtemplates/apple/Sources/Channel.swift.twig
📚 Learning: 2025-12-18T18:43:35.419Z
Learnt from: ArnabChatterjee20k
Repo: appwrite/sdk-generator PR: 1266
File: templates/apple/Sources/Channel.swift.twig:13-19
Timestamp: 2025-12-18T18:43:35.419Z
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:
templates/apple/Sources/Channel.swift.twig
⏰ 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). (17)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: node (server)
- GitHub Check: swift (server)
- GitHub Check: build (8.3, Python313)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, Node18)
- GitHub Check: cli (console)
- GitHub Check: flutter (client)
- GitHub Check: build (8.3, Python39)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, DotNet80)
- GitHub Check: android (client)
- GitHub Check: react-native (client)
- GitHub Check: build (8.3, Android5Java17)
🔇 Additional comments (6)
tests/AppleSwift56Test.php (2)
19-19: LGTM! Improved error capture.The addition of
2>&1to capture both stdout and stderr is a good improvement for debugging compilation errors during SDK testing.
34-34: Addition of CHANNEL_HELPER_RESPONSES is correct and complete.The constant
Base::CHANNEL_HELPER_RESPONSESis properly defined in the Base test class, and AppleSwift56Test.php correctly includes it alongside other platform test files that support channel helpers (Web, Flutter, and Android). No additional updates are required.templates/apple/Sources/Channel.swift.twig (1)
169-179: LGTM! Thevalue:argument label fix has been applied.The ResolvedChannel initializations now correctly include the
value:argument label, resolving the compilation error flagged in the previous review.templates/apple/Sources/Services/Realtime.swift.twig (3)
120-125: LGTM! Clean abstraction for channel conversion.The
channelToStringhelper provides a clear conversion point from the publicChannelValueprotocol to the internalStringrepresentation used by the WebSocket layer.
161-171: Good addition: typed multi-channel subscription with ChannelValue.This new overload completes the API surface by allowing subscriptions to multiple channels with a specific payload type using
ChannelValue. The conversion from[ChannelValue]toSet<String>is consistent with other overloads and properly removes duplicates.
227-227: LGTM! Explicit NIO.Channel qualification avoids naming conflict.The explicit
NIO.Channelqualification prevents ambiguity with the newChannelhelper enum for realtime subscriptions. This is the same type, just explicitly scoped.Based on learnings, this qualification pattern is intentional to avoid naming collisions.
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
API Changes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.