-
Couldn't load subscription status.
- Fork 34
feat: Apple SDK update for version 13.3.0 #93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis pull request releases version 13.3.0 of the Appwrite Apple SDK. The changes include updating the version number in CHANGELOG.md, README.md, and the Client.swift SDK version header. The primary functional addition introduces three new public callback registration methods to the Realtime service class: Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes The diff consists predominantly of straightforward version bumps across documentation and configuration files, which require minimal review. The substantive changes in Realtime.swift follow a consistent, repetitive pattern—three identical callback registration methods that each store and invoke closures. The integration points are clear and localized to existing WebSocket lifecycle event handlers. The logic density is low, and the changes do not introduce complex control flow or error handling beyond the callback pattern itself. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/Appwrite/Services/Realtime.swift (1)
30-40: Consider adding callback unregistration support.The current API allows registering callbacks but provides no mechanism to unregister them. This could lead to unintended callback retention throughout the Realtime instance's lifetime.
Consider returning a subscription token or adding corresponding removal methods if clients need to manage callback lifecycles dynamically.
Example pattern:
public func onError(_ callback: @escaping (Swift.Error?, HTTPResponseStatus?) -> Void) -> CallbackToken { let token = CallbackToken() connectSync.sync { self.onErrorCallbacks[token.id] = callback } return token } public func removeCallback(token: CallbackToken) { connectSync.sync { self.onErrorCallbacks.removeValue(forKey: token.id) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)README.md(1 hunks)Sources/Appwrite/Client.swift(1 hunks)Sources/Appwrite/Services/Realtime.swift(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Sources/Appwrite/Services/Realtime.swift (1)
Sources/Appwrite/WebSockets/WebSocketClient.swift (3)
onError(172-174)onClose(164-166)onOpen(140-142)
🔇 Additional comments (4)
Sources/Appwrite/Client.swift (1)
26-26: LGTM!The SDK version header update to 13.3.0 is correct and aligns with the PR objectives.
README.md (1)
34-34: LGTM!The package dependency version update is correct and maintains consistency with the SDK version bump.
CHANGELOG.md (1)
3-6: LGTM!The changelog entry accurately documents the new Realtime callback functionality and follows the existing format.
Sources/Appwrite/Services/Realtime.swift (1)
208-212: LGTM with caveat!The callback invocation at the appropriate WebSocket lifecycle event is correct. However, ensure thread safety is addressed for the callback array access as noted in the earlier comment.
| private var onErrorCallbacks: [((Swift.Error?, HTTPResponseStatus?) -> Void)] = [] | ||
| private var onCloseCallbacks: [(() -> Void)] = [] | ||
| private var onOpenCallbacks: [(() -> Void)] = [] | ||
|
|
||
| public func onError(_ callback: @escaping (Swift.Error?, HTTPResponseStatus?) -> Void) { | ||
| self.onErrorCallbacks.append(callback) | ||
| } | ||
|
|
||
| public func onClose(_ callback: @escaping () -> Void) { | ||
| self.onCloseCallbacks.append(callback) | ||
| } | ||
|
|
||
| public func onOpen(_ callback: @escaping () -> Void) { | ||
| self.onOpenCallbacks.append(callback) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider thread safety for callback storage and registration.
The callback arrays are accessed without synchronization. If onOpen, onClose, or onError registration methods are called concurrently with callback invocations (lines 210, 231, 253), or if multiple threads register callbacks simultaneously, this could lead to race conditions.
Consider using the existing connectSync queue or introducing dedicated synchronization for callback array access.
Apply this pattern to protect array mutations:
public func onError(_ callback: @escaping (Swift.Error?, HTTPResponseStatus?) -> Void) {
- self.onErrorCallbacks.append(callback)
+ connectSync.sync {
+ self.onErrorCallbacks.append(callback)
+ }
}Apply similar synchronization to onClose and onOpen methods, and protect the forEach invocations with the same queue.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Sources/Appwrite/Services/Realtime.swift around lines 26 to 40, the callback
arrays (onErrorCallbacks, onCloseCallbacks, onOpenCallbacks) are mutated and
read without synchronization which can cause race conditions; protect all
mutations (append) and iterations (forEach invocations at lines ~210, 231, 253)
by using the existing connectSync dispatch queue (or a dedicated serial
DispatchQueue) to serialize access: wrap appends in connectSync.async or sync as
appropriate, and wrap callback invocations in connectSync.sync/async while
copying the array to a local constant before iteration to avoid holding the
queue during callbacks. Ensure the same queue is used for registration and
invocation for each callback array to guarantee thread safety.
This PR contains updates to the Apple SDK for version 13.3.0.
Summary by CodeRabbit