feat: add MMKV integration#21
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an async storage abstraction with MMKV and migration from UserDefaults, makes SDK initialization asynchronous and conditional for app extensions, updates app group identifier resolution to use bundleId, bumps version to 1.6.0, adds CocoaPods sample, updates samples and workspace/configs, and documents changes including iOS 26 simulator note. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sk and error handling
pitzcarraldo
force-pushed
the
feat/mmkv
branch
from
ebfa398 to
b9a5b71
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (4)
Samples/CocoaPods/Resources (1)
1-1: Same issue as ClixNotificationExtension.This file has the same problem - it contains a path string rather than being a proper reference mechanism. Please see the comment on
Samples/CocoaPods/ClixNotificationExtensionfor details.Samples/CocoaPods/BasicApp-Info.plist (1)
1-1: Same issue as ClixNotificationExtension.This file has the same problem - it contains a path string rather than being a proper reference mechanism. Please see the comment on
Samples/CocoaPods/ClixNotificationExtensionfor details.Samples/CocoaPods/BasicApp.entitlements (1)
1-1: Same issue as ClixNotificationExtension.This file has the same problem - it contains a path string rather than being a proper reference mechanism. Please see the comment on
Samples/CocoaPods/ClixNotificationExtensionfor details.Sources/Storage/StorageTypeMigrator.swift (1)
18-55: Migration doesn't handle Codable structs stored as JSON-encoded Data.The typed
get<T: Codable>calls at lines 20-31 and 40-55 attempt to decode stored values asString,[String], orData. However, keys likeclix_configandclix_notification_settingsstore Codable structs as JSON-encodedData. When callingsource.get<Data>(key), the decoder attempts to deserialize the JSON as theDatatype, which fails because the underlying JSON represents a struct, not raw Data.Consider using raw data access similar to
StorageMigrator:private static func migrateData(from source: UserDefaultsStorage, to destination: MmkvStorage) async { var migratedCount = 0 for key in StorageMigrator.knownStorageKeys { - if let stringValue: String = await source.get(key) { - await destination.set(key, stringValue) - await source.remove(key) - migratedCount += 1 - ClixLogger.debug("Migrated key: \(key)") - } else if let arrayValue: [String] = await source.get(key) { - ... - } + // Access raw Data directly from UserDefaults instead of going through Codable decode + if let rawData = await source.getRawData(key) { + await destination.setRawData(key, rawData) + await source.remove(key) + migratedCount += 1 + ClixLogger.debug("Migrated key: \(key)") + } } ... }This requires adding
getRawData(_:)toUserDefaultsStoragesimilar toMmkvStorage.
🧹 Nitpick comments (14)
README.md (1)
603-621: iOS 26 simulator note is clear; consider cross‑linking to checklistThe dedicated section explains the simulator‑only APNs issue and workaround well. Optionally, you could add a short cross‑reference to this section from the earlier debugging checklist item about “iOS 26 simulator” to avoid readers missing the detailed explanation.
Clix.podspec (1)
4-20: Confirm MMKV version range and extension build settingThe 1.6.0 version bump and added MMKV dependency look reasonable. Please double‑check that:
- The MMKV version range here matches whatever you declare for SPM (if any), and
- Setting
APPLICATION_EXTENSION_API_ONLYtoNOis intentional given your extension‑compatibility goals (e.g., that app extensions only rely on extension‑safe APIs in this pod).Aligning these between package managers helps avoid subtle divergence for consumers.
Sources/Storage/Storage.swift (1)
3-10: Storage actor protocol shape looks good; consider addingSendableThe actor‑based
Storageprotocol with Codable generics lines up well withMmkvStorageandUserDefaultsStorage, and the async wrappers inStorageServiceuse it correctly.As a future‑proofing refinement, you might consider constraining the generic to
T: Codable & Sendableso values crossing the actor boundary are guaranteed to be concurrency‑safe and newer Swift versions won’t emit warnings for non‑Sendable types.Sources/Utils/BundleIdentifier.swift (1)
3-62: Consider distinguishing “no app group configured” from legacy projectId fallbackThe bundleId/app‑extension resolution logic and app‑group ID builders look solid, and preferring
group.clix.{bundleId}overgroup.clix.{projectId}matches the migration story.One subtle point: when neither identifier has an available container,
availableAppGroupIdlogs a warning but still returnsprojectIdBased, so callers can’t tell “no app group configured at all” from “using legacy projectId‑based group”. If you ever need to react differently to a missing configuration (e.g., skip MMKV init or show a clearer error), consider exposing that case explicitly (e.g., returningString?or adding a separate helper that can returnnil/throw) rather than always returning a non‑existent ID.Samples/BasicApp/ClixConfig.json.example (1)
1-6: Consider adding logLevel to the example configuration.The example configuration is well-structured, but it might be helpful to include a
logLevelfield (e.g.,"logLevel": "info") since the SDK likely supports configurable logging levels.Example addition:
{ "projectId": "your-project-id", "apiKey": "your-api-key", "endpoint": "https://api.clix.so", + "logLevel": "info", "extraHeaders": {} }Sources/Notification/ClixNotificationServiceExtension.swift (1)
40-45: Consider guarding against multipleregister()calls.If
register()is called multiple times (e.g., ifdidReceiveis invoked repeatedly before the first initialization completes), theinitializationTaskgets overwritten, potentially creating orphaned tasks.open func register(projectId: String) { + guard initializationTask == nil else { + ClixLogger.debug("SDK already initializing, skipping duplicate register call") + return + } initializationTask = Task { try await Clix.initialize(projectId: projectId) ClixLogger.info("Registered with project ID: \(projectId)") } }Samples/CocoaPods/README.md (1)
109-120: Consider adding a language specifier to the code block.The project structure code block could use a language specifier (e.g.,
textorplaintext) to satisfy markdown linting, though this is optional for directory trees.-``` +```text CocoaPods/ ├── Sources/ # Symlink to ../BasicApp/SourcesSources/Core/ClixNotification.swift (1)
12-12: Consider thread-safety for the initialization guard.The
isInitializedflag is a non-atomicBoolchecked and set without synchronization. Ifconfigure()is called concurrently from multiple threads (rare but possible during async initialization), both could pass the guard before either sets the flag.Given that
ClixNotificationis a singleton typically configured once at app startup, this is a low-probability scenario. If you want to harden this, consider using an atomic flag or a dispatch-once pattern.Also applies to: 32-46
Samples/CocoaPods/BasicApp.xcodeproj/project.pbxproj (1)
569-569: Minor: Unconventional extension bundle identifier pattern.The extension uses
so.clix.samples.basic.app.ClixNotificationExtensionwhich includes.app.between the main app ID and extension name. Apple's convention typically omits this:so.clix.samples.basic.ClixNotificationExtension.This works fine but may cause minor confusion for developers following Apple's documentation. Consider aligning with the standard pattern if this sample will serve as reference documentation.
Also applies to: 596-596
Sources/Storage/MmkvStorage.swift (2)
42-47: Force unwrap risk in final fallback.If all MMKV initialization attempts fail (including
MMKV.default()), the force unwrap on line 46 will crash. While this scenario is unlikely after callingMMKV.initialize(), it's a potential crash point if MMKV initialization fails silently.Consider a defensive approach:
} else { ClixLogger.error("Critical: All MMKV initialization failed, using temporary fallback") - self.mmkv = - MMKV(mmapID: "clix.temporary.\(UUID().uuidString)", mode: .singleProcess) - ?? MMKV.default()! + guard let fallback = MMKV(mmapID: "clix.temporary.\(UUID().uuidString)", mode: .singleProcess) + ?? MMKV.default() else { + fatalError("MMKV initialization completely failed - cannot proceed") + } + self.mmkv = fallback }This makes the intentional crash explicit with a descriptive message rather than an implicit force-unwrap crash.
76-89: Consider reducing log verbosity forgetWithRetry.The
infolog at line 87 fires on every retrieval, which could be noisy in production. Consider usingdebuglevel instead:- ClixLogger.info("Retrieved value source: \(value != nil ? "stored" : "fallback")") + ClixLogger.debug("Retrieved value source: \(value != nil ? "stored" : "fallback")")Samples/BasicApp/Sources/ClixConfiguration.swift (1)
29-29: Consider reading logLevel from cached configuration.The
logLevelproperty is hardcoded to.debugwhile other properties (projectId, apiKey, endpoint, extraHeaders) read from thecacheddictionary. For consistency and flexibility, consider reading logLevel from the JSON configuration if present.Apply this diff if you'd like to make logLevel configurable:
- public lazy var logLevel: ClixLogLevel = .debug + public lazy var logLevel: ClixLogLevel = { + if let level = cached["logLevel"] as? String { + return ClixLogLevel(rawValue: level) ?? .debug + } + return .debug + }()Sources/Storage/UserDefaultsStorage.swift (2)
20-28: Log encoding failures for better observability.The
setmethod silently ignores encoding failures withtry?. Consider logging when encoding fails to aid debugging, as this could indicate issues with non-Codable types or data corruption.Apply this diff:
func set<T: Codable>(_ key: String, _ value: T?) { if let value = value { if let encoded = try? encoder.encode(value) { userDefaults.set(encoded, forKey: key) + } else { + ClixLogger.error("Failed to encode value for key: \(key)") } } else { userDefaults.removeObject(forKey: key) } }
30-35: Log decoding failures for better observability.The
getmethod silently returnsnilon decoding failures withtry?. Consider logging decode failures to help diagnose data corruption or type mismatches.Apply this diff:
func get<T: Codable>(_ key: String) -> T? { guard let data = userDefaults.data(forKey: key) else { return nil } - return try? decoder.decode(T.self, from: data) + do { + return try decoder.decode(T.self, from: data) + } catch { + ClixLogger.debug("Failed to decode value for key \(key): \(error)") + return nil + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Samples/CocoaPods/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (48)
.github/workflows/release.yml(2 hunks).gitignore(1 hunks).sourcekit-lsp/config.json(1 hunks)AGENTS.md(1 hunks)CHANGELOG.md(1 hunks)CLAUDE.md(1 hunks)Clix.podspec(2 hunks)Clix.xcworkspace/contents.xcworkspacedata(0 hunks)Package.swift(1 hunks)README.md(2 hunks)Samples/BasicApp/BasicApp.entitlements(1 hunks)Samples/BasicApp/BasicApp.xcodeproj/project.pbxproj(5 hunks)Samples/BasicApp/BasicApp.xcodeproj/xcshareddata/xcschemes/BasicApp.xcscheme(1 hunks)Samples/BasicApp/BasicApp.xcworkspace/contents.xcworkspacedata(1 hunks)Samples/BasicApp/BasicApp.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings(1 hunks)Samples/BasicApp/ClixConfig.json.example(1 hunks)Samples/BasicApp/ClixNotificationExtension/ClixNotificationExtension.entitlements(1 hunks)Samples/BasicApp/ClixNotificationExtension/NotificationService.swift(1 hunks)Samples/BasicApp/README.md(1 hunks)Samples/BasicApp/Sources/AppDelegate.swift(3 hunks)Samples/BasicApp/Sources/AppState.swift(0 hunks)Samples/BasicApp/Sources/ClixConfiguration.swift(1 hunks)Samples/BasicApp/Sources/ContentView.swift(1 hunks)Samples/BasicApp/Sources/SplashView.swift(0 hunks)Samples/CocoaPods/BasicApp-Info.plist(1 hunks)Samples/CocoaPods/BasicApp.entitlements(1 hunks)Samples/CocoaPods/BasicApp.xcodeproj/project.pbxproj(1 hunks)Samples/CocoaPods/BasicApp.xcodeproj/project.xcworkspace/contents.xcworkspacedata(1 hunks)Samples/CocoaPods/BasicApp.xcodeproj/xcshareddata/xcschemes/BasicApp.xcscheme(1 hunks)Samples/CocoaPods/BasicApp.xcodeproj/xcshareddata/xcschemes/ClixNotificationExtension.xcscheme(1 hunks)Samples/CocoaPods/BasicApp.xcworkspace/contents.xcworkspacedata(1 hunks)Samples/CocoaPods/ClixNotificationExtension(1 hunks)Samples/CocoaPods/Podfile(1 hunks)Samples/CocoaPods/README.md(1 hunks)Samples/CocoaPods/Resources(1 hunks)Samples/CocoaPods/Sources(1 hunks)Sources/Core/Clix.swift(5 hunks)Sources/Core/ClixNotification.swift(2 hunks)Sources/Core/ClixVersion.swift(1 hunks)Sources/Notification/ClixNotificationServiceExtension.swift(2 hunks)Sources/Services/StorageService.swift(1 hunks)Sources/Storage/MmkvStorage.swift(1 hunks)Sources/Storage/Storage.swift(1 hunks)Sources/Storage/StorageInitializer.swift(1 hunks)Sources/Storage/StorageMigrator.swift(1 hunks)Sources/Storage/StorageTypeMigrator.swift(1 hunks)Sources/Storage/UserDefaultsStorage.swift(1 hunks)Sources/Utils/BundleIdentifier.swift(1 hunks)
💤 Files with no reviewable changes (3)
- Samples/BasicApp/Sources/AppState.swift
- Clix.xcworkspace/contents.xcworkspacedata
- Samples/BasicApp/Sources/SplashView.swift
🧰 Additional context used
🧬 Code graph analysis (10)
Sources/Storage/Storage.swift (4)
Sources/Storage/MmkvStorage.swift (7)
set(51-59)get(61-66)getWithRetry(76-89)getRawData(68-70)setRawData(72-74)remove(91-93)synchronize(95-97)Sources/Storage/UserDefaultsStorage.swift (7)
set(20-28)get(30-35)getWithRetry(45-57)getRawData(37-39)setRawData(41-43)remove(59-61)synchronize(63-65)Sources/Services/StorageService.swift (5)
set(10-12)get(14-16)getWithRetry(18-20)remove(22-24)synchronize(26-28)Sources/Core/Clix.swift (1)
get(126-133)
Sources/Notification/ClixNotificationServiceExtension.swift (3)
Sources/Utils/Logging/ClixLogger.swift (2)
debug(50-52)error(38-40)Sources/Core/Clix.swift (1)
getWithWait(136-139)Sources/Services/NotificationService.swift (3)
handlePushReceived(27-54)processNotificationWithImage(158-176)processNotificationWithImage(178-186)
Sources/Storage/StorageInitializer.swift (4)
Sources/Utils/BundleIdentifier.swift (2)
bundleIdBasedAppGroupId(13-15)projectIdBasedAppGroupId(17-19)Sources/Storage/StorageTypeMigrator.swift (1)
migrateIfNeeded(4-16)Sources/Storage/MmkvStorage.swift (6)
getRawData(68-70)setRawData(72-74)remove(91-93)synchronize(95-97)get(61-66)set(51-59)Sources/Storage/UserDefaultsStorage.swift (6)
getRawData(37-39)setRawData(41-43)remove(59-61)synchronize(63-65)get(30-35)set(20-28)
Sources/Services/StorageService.swift (3)
Sources/Storage/StorageInitializer.swift (1)
initializeStorage(17-32)Sources/Storage/MmkvStorage.swift (5)
set(51-59)get(61-66)getWithRetry(76-89)remove(91-93)synchronize(95-97)Sources/Storage/UserDefaultsStorage.swift (5)
set(20-28)get(30-35)getWithRetry(45-57)remove(59-61)synchronize(63-65)
Sources/Storage/StorageTypeMigrator.swift (5)
Sources/Utils/Logging/ClixLogger.swift (2)
debug(50-52)info(46-48)Sources/Core/Clix.swift (1)
get(126-133)Sources/Storage/MmkvStorage.swift (4)
get(61-66)set(51-59)remove(91-93)synchronize(95-97)Sources/Storage/UserDefaultsStorage.swift (4)
get(30-35)set(20-28)remove(59-61)synchronize(63-65)Sources/Services/StorageService.swift (4)
get(14-16)set(10-12)remove(22-24)synchronize(26-28)
Sources/Storage/StorageMigrator.swift (4)
Sources/Utils/Logging/ClixLogger.swift (2)
debug(50-52)info(46-48)Sources/Storage/MmkvStorage.swift (1)
set(51-59)Sources/Storage/UserDefaultsStorage.swift (1)
set(20-28)Sources/Services/StorageService.swift (1)
set(10-12)
Samples/BasicApp/ClixNotificationExtension/NotificationService.swift (1)
Sources/Notification/ClixNotificationServiceExtension.swift (1)
register(40-45)
Sources/Storage/UserDefaultsStorage.swift (4)
Sources/Utils/Logging/ClixLogger.swift (2)
debug(50-52)info(46-48)Sources/Storage/MmkvStorage.swift (5)
set(51-59)get(61-66)getWithRetry(76-89)remove(91-93)synchronize(95-97)Sources/Services/StorageService.swift (5)
set(10-12)get(14-16)getWithRetry(18-20)remove(22-24)synchronize(26-28)Sources/Core/Clix.swift (1)
get(126-133)
Sources/Utils/BundleIdentifier.swift (1)
Sources/Utils/Logging/ClixLogger.swift (1)
warn(42-44)
Samples/BasicApp/Sources/AppDelegate.swift (2)
Sources/Core/Clix.swift (3)
initialize(152-175)initialize(186-190)initialize(192-196)Sources/Core/ClixNotification.swift (1)
initialize(32-46)
🪛 LanguageTool
AGENTS.md
[style] ~191-~191: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...mportant**: Both targets must use the exact same App Group identifier for proper data ...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
🪛 markdownlint-cli2 (0.18.1)
Samples/CocoaPods/README.md
109-109: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 RuboCop (1.81.7)
Samples/CocoaPods/Podfile
[convention] 10-11: Extra empty line detected at block body end.
(Layout/EmptyLinesAroundBlockBody)
[convention] 19-20: Extra empty line detected at block body end.
(Layout/EmptyLinesAroundBlockBody)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (41)
.github/workflows/release.yml (2)
17-18: Explicit ref specification improves clarity.The addition of
ref: ${{ github.ref }}is a good practice for release workflows, making explicit which ref is being checked out. Forworkflow_dispatchevents, this ensures the correct branch context is used.
54-54: Commit SHA association strengthens release integrity.Adding
commit: ${{ github.sha }}to the release action ensures the GitHub release is explicitly tied to the commit where the version was updated. Combined with the version validation step (lines 20-33), this creates a strong safety barrier against releasing from incorrect commits.Samples/BasicApp/BasicApp.xcworkspace/contents.xcworkspacedata (1)
1-7: LGTM!Standard Xcode workspace file with correct structure and formatting.
Samples/BasicApp/BasicApp.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings (1)
7-8: LGTM!The
IDEWorkspaceSharedSettings_AutocreateContextsIfNeededsetting is appropriate for a shared workspace, providing deterministic scheme management.Samples/BasicApp/BasicApp.xcodeproj/xcshareddata/xcschemes/BasicApp.xcscheme (1)
5-78: LGTM! Standard Xcode scheme configuration.The scheme configuration is well-structured with appropriate settings:
- Build actions properly configured for all scenarios
- Debug configuration for development (Test, Launch, Analyze)
- Release configuration for distribution (Profile, Archive)
- Shared scheme location ensures consistent builds across environments
Samples/CocoaPods/BasicApp.xcodeproj/project.xcworkspace/contents.xcworkspacedata (1)
1-7: Standard Xcode workspace configuration.This is standard boilerplate for Xcode workspace configuration with no issues.
.sourcekit-lsp/config.json (1)
3-3: SourceKit LSP now targets simulator.The triple has been changed from
arm64-apple-iostoarm64-apple-ios-simulator. This will configure the language server for simulator builds, which may affect IDE features when working with physical device-specific code.Ensure the team is aware that SourceKit LSP is now configured specifically for simulator targets. Developers working primarily on physical devices may experience different IDE behavior.
Samples/CocoaPods/ClixNotificationExtension (1)
1-1: The files are already properly configured as symbolic links and are functioning correctly. All four files (ClixNotificationExtension, Resources, BasicApp-Info.plist, and BasicApp.entitlements) are symlinks pointing to their corresponding files in../BasicApp/as intended. No action is required.Likely an incorrect or invalid review comment.
CLAUDE.md (1)
1-1: Remove this comment — CLAUDE.md is a valid symlink with complete documentation.CLAUDE.md is correctly implemented as a symlink to AGENTS.md (9 bytes → 9887 bytes of content). Both files serve the same purpose: providing comprehensive guidance to Claude Code and other agents when working with the Clix iOS SDK repository. The symlink allows the documentation to be referenced by multiple names while maintaining a single source of truth with 254 lines of substantive content covering project overview, architecture, integration patterns, and development commands.
Sources/Core/ClixVersion.swift (1)
5-5: Version bumped correctly to 1.6.0The version constant aligns with the 1.6.0 release notes and packaging metadata; no further changes needed here.
README.md (1)
11-11: SPM dependency version matches 1.6.0 releaseThe
from: "1.6.0"constraint is consistent with the new SDK version and changelog.Samples/CocoaPods/BasicApp.xcodeproj/xcshareddata/xcschemes/ClixNotificationExtension.xcscheme (1)
1-97: Extension scheme wiring looks consistent with common patternsThe scheme builds both the notification extension and the BasicApp host, and uses the app as the runnable, which fits the usual extension debugging setup. Nothing obviously problematic here.
CHANGELOG.md (1)
7-32: 1.6.0 changelog entry is clear and matches the code changesThe description of storage (MMKV + migrations), app‑group identifier changes, and app‑extension behavior matches the implementation direction in this PR and gives consumers a good overview.
AGENTS.md (1)
1-253: Internal agent guidance doc is consistent and helpfulThis documentation accurately reflects the current architecture (storage abstraction with MMKV, app‑group conventions, initialization and notification flows) and should be useful for automated tooling; nothing blocking here.
.gitignore (1)
13-14: LGTM!Appropriate additions to ignore local configuration and CocoaPods-generated dependencies.
Samples/CocoaPods/Sources (1)
1-1: LGTM!Appropriate reference to share BasicApp sources with the CocoaPods sample, avoiding code duplication.
Samples/BasicApp/Sources/ContentView.swift (1)
36-36: LGTM!Consistent migration to the singleton configuration pattern.
Also applies to: 40-40
Samples/BasicApp/Sources/AppDelegate.swift (3)
2-2: LGTM!Correct import change from
FirebaseAuthtoFirebaseCorefor theFirebaseApp.configure()call.
58-58: Removed error handler for push notification registration failures.The override for
application(_:didFailToRegisterForRemoteNotificationsWithError:)has been removed. This means push notification registration errors will no longer be logged or handled in the sample app.Verify this is intentional, as error handling for push registration failures can be valuable for debugging.
16-16: Fire-and-forget initialization is intentionally designed and properly handled.The SDK correctly uses the synchronous
initialize()overload (lines 186–189 of Clix.swift), which wraps the async initialization in aTaskfor non-blocking startup. All SDK methods that are called later—includingsetUserId(line 20) andtrackEvent—properly wait for initialization completion through theInitCoordinatorbefore proceeding. TheInitCoordinatoractor manages initialization state and usesCheckedContinuationto ensure waiting async calls block untilcompleteInitialization()is called. This pattern is well-designed and no changes are needed.Samples/CocoaPods/BasicApp.xcworkspace/contents.xcworkspacedata (1)
1-10: LGTM!Standard CocoaPods workspace structure with correct project references.
Samples/BasicApp/ClixNotificationExtension/ClixNotificationExtension.entitlements (1)
7-7: Verify app group identifier consistency across SDK initialization.The app group identifier has been updated from a projectId-based format to a bundleId-based format (
group.clix.so.clix.samples.basic). Ensure that the SDK's storage initialization (particularly MMKV) uses the same app group identifier. This requires manual verification of the SDK initialization code to confirm the identifiers match.Samples/BasicApp/ClixNotificationExtension/NotificationService.swift (1)
7-12: Synchronization is properly implemented; no action required.The
didReceive()method in the parent classClixNotificationServiceExtensionproperly waits for the initialization task to complete. At line 61–64 ofClixNotificationServiceExtension.swift, the code checks for theinitializationTaskand awaits its completion withtry await initTask.valuebefore processing the notification. This ensures the SDK is fully initialized before the notification is handled, even if a notification arrives immediately after the extension launches.Samples/BasicApp/BasicApp.entitlements (1)
9-9: LGTM!The updated app group identifier follows a clear naming convention derived from the bundle identifier, which aligns with the PR's approach to use bundleId-based app group identifiers for better consistency.
Samples/CocoaPods/Podfile (1)
1-20: LGTM!The Podfile is correctly configured with appropriate platform version,
use_frameworks!for Swift compatibility, and local path reference for development. Both targets properly declare the Clix dependency.Sources/Notification/ClixNotificationServiceExtension.swift (1)
59-76: Async initialization gating looks good.The pattern of awaiting the initialization task before processing notifications ensures the SDK is ready. The error handling correctly falls back to delivering the original content if initialization or processing fails.
Samples/CocoaPods/BasicApp.xcodeproj/xcshareddata/xcschemes/BasicApp.xcscheme (1)
1-78: LGTM!Standard Xcode scheme configuration for the CocoaPods sample project with appropriate build configurations for each action.
Samples/BasicApp/README.md (1)
1-223: Comprehensive documentation.The README provides thorough setup instructions, project structure, implementation examples, and troubleshooting guidance. Well done!
Samples/CocoaPods/README.md (1)
1-315: Thorough CocoaPods documentation.Good coverage of setup steps, comparison with SPM, troubleshooting, and the symlink-based code sharing approach.
Samples/BasicApp/BasicApp.xcodeproj/project.pbxproj (1)
552-555: LGTM!The local Swift package reference path update from
../../../clix-ios-sdkto../../correctly reflects the new project structure within the repository.Sources/Core/ClixNotification.swift (1)
48-55: LGTM!The
configure()refactoring correctly sequences state assignment before initialization and conditionally requests permission after setup is complete.Samples/CocoaPods/BasicApp.xcodeproj/project.pbxproj (1)
1-638: LGTM!The CocoaPods sample project is well-structured with proper build phases for Pod manifest verification, framework embedding, and appropriate build settings including
ENABLE_USER_SCRIPT_SANDBOXING = NOfor CocoaPods compatibility.Sources/Storage/StorageMigrator.swift (2)
4-51: LGTM!The generic
migrateKeysmethod is well-designed. Using rawDataaccess correctly preserves Codable-encoded values without requiring type information at migration time. The closure-based approach provides good flexibility for different storage backends.
53-89: LGTM!The convenience methods for MMKV-to-MMKV and UserDefaults-to-UserDefaults migration are clean implementations that correctly leverage the generic
migrateKeysmethod with appropriate closures for each storage type.Sources/Storage/MmkvStorage.swift (1)
4-98: LGTM overall!The actor-based MMKV storage implementation is well-structured with:
- Proper app group detection and mode selection
- Defensive fallback initialization paths
- Raw data access methods for migration support
- Retry mechanism with fallback for eventual consistency scenarios
Sources/Services/StorageService.swift (1)
3-28: LGTM! Clean refactoring to storage abstraction.The conversion from direct UserDefaults usage to a generic Storage abstraction is well-executed. The actor-based synchronization, async initialization, and consistent delegation pattern provide a clean separation of concerns.
Sources/Core/Clix.swift (3)
76-78: LGTM! Proper app extension API guard.The conditional compilation directive correctly excludes the
NotificationAPI in application extension contexts where UNUserNotificationCenter APIs may not be available.
97-118: LGTM! Async storage initialization integrated correctly.The
setConfigmethod now properly awaits asynchronous storage initialization viaStorageService(projectId:), aligning with the new storage abstraction. The service initialization chain is well-structured with appropriate error logging at each step.
152-175: LGTM! Consistent async initialization flow.The async
initializemethod correctly:
- Awaits storage configuration (line 155)
- Waits for all services to be ready before environment setup
- Guards notification initialization for app extensions (lines 167-169)
- Signals completion via initCoordinator (line 171)
The synchronization ensures all public APIs called after initialization have access to fully initialized services.
Sources/Storage/StorageInitializer.swift (2)
225-253: Migration safety confirmed: all critical storage keys are included.The
migrateAndDeleteFromSourcemethod is safe. All data keys that need to be preserved are included inStorageMigrator.knownStorageKeys:
- clix_config
- clix_device_id
- clix_current_push_token
- clix_push_tokens
- clix_notification_settings
- clix_last_received_message_id
The only omitted key is
clix_storage_migrated(the migration flag), which is intentionally not migrated as it's metadata tracking the migration state, not user data. Deletion after successful migration is safe and correct.
17-94: Migration behavior is intentional but warrants documentation.The migration flag is persisted after attempting all sources, not after confirming success from each source. This design means:
- Skipped sources (unavailable app groups) are not retried in subsequent launches
- Individual migration failures are not retried, though missing keys don't cause errors
- The
knownStorageKeyslist comprehensively covers all SDK storage keysThis is safe if all sources are checked in a single pass (which they are: projectId-based MMKV/UserDefaults, bundleId-based UserDefaults, and standard UserDefaults). However, if a source temporarily becomes unavailable during the first migration attempt, its data will not be migrated on subsequent launches. Consider whether this edge case needs handling via explicit error logging or a retriable state if a source fails unexpectedly.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Changed
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.