Use koin annotations in persistence module #126
Conversation
WalkthroughThis change set introduces a comprehensive refactor and enhancement of the persistence and authentication flow in a Kotlin Multiplatform project. The persistence module's dependency injection setup is migrated from manual Koin module definitions to annotation-driven modules using Koin's annotation processing. Platform-specific DataStore provisioning is now handled by dedicated Sequence Diagram(s)User Login State FlowsequenceDiagram
participant UI_LoginComponent
participant SetUserLoggedInUseCase
participant UserPersistence
participant PrimitivePersistence
UI_LoginComponent->>SetUserLoggedInUseCase: execute(Args(isLoggedIn=true))
SetUserLoggedInUseCase->>UserPersistence: setUserLoggedIn(true)
UserPersistence->>PrimitivePersistence: putBoolean("IS_USER_LOGGED_IN", true)
PrimitivePersistence-->>UserPersistence: (persisted)
UserPersistence-->>SetUserLoggedInUseCase: (complete)
SetUserLoggedInUseCase-->>UI_LoginComponent: (success)
UI_LoginComponent->>UI_LoginComponent: navigateToSignedIn()
User Logout State FlowsequenceDiagram
participant UI_ProfileComponent
participant SetUserLoggedInUseCase
participant UserPersistence
participant PrimitivePersistence
UI_ProfileComponent->>SetUserLoggedInUseCase: execute(Args(isLoggedIn=false))
SetUserLoggedInUseCase->>UserPersistence: setUserLoggedIn(false)
UserPersistence->>PrimitivePersistence: putBoolean("IS_USER_LOGGED_IN", false)
PrimitivePersistence-->>UserPersistence: (persisted)
UserPersistence-->>SetUserLoggedInUseCase: (complete)
SetUserLoggedInUseCase-->>UI_ProfileComponent: (success)
UI_ProfileComponent->>UI_ProfileComponent: navigateToLogin()
App Startup: Root Navigation DecisionsequenceDiagram
participant RootNavHostComponent
participant IsUserLoggedInUseCase
participant UserPersistence
participant PrimitivePersistence
RootNavHostComponent->>IsUserLoggedInUseCase: execute(Unit)
IsUserLoggedInUseCase->>UserPersistence: isUserLoggedIn()
UserPersistence->>PrimitivePersistence: getBoolean("IS_USER_LOGGED_IN")
PrimitivePersistence-->>UserPersistence: Boolean value
UserPersistence-->>IsUserLoggedInUseCase: Boolean value
IsUserLoggedInUseCase-->>RootNavHostComponent: Boolean value
alt User is logged in
RootNavHostComponent->>RootNavHostComponent: activate(SignedIn)
else User is not logged in
RootNavHostComponent->>RootNavHostComponent: activate(Login)
end
Persistence Module Initialization (Platform-specific)sequenceDiagram
participant Koin
participant PersistenceModule
participant DataStoreModule (Android/iOS)
participant DataStore
Koin->>PersistenceModule: Instantiate
PersistenceModule->>DataStoreModule: (platform-specific instantiation)
DataStoreModule->>DataStore: provideDataStore()
DataStore-->>DataStoreModule: DataStore<Preferences>
DataStoreModule-->>PersistenceModule: DataStore<Preferences>
PersistenceModule-->>Koin: register components
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
convention-plugins/src/main/kotlin/conventions-annotations-processor.gradle.kts (1)
35-38: Consider adding the JVM target or parametrising the listOnly Android & iOS source sets are wired for KSP.
If the project ever adds a pure JVM target (desktop, server), the processor will be missing.
A small helper avoids future drift:listOf( "kspCommonMainMetadata", "kspAndroid", "kspIosX64", "kspIosArm64", "kspIosSimulatorArm64", "kspJvm" // ← future-proof ).forEach { conf -> add(conf, libs.koin.ksp.compiler) }shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/persistence/PrimitivePersistence.kt (1)
15-17:@Providedis unnecessary noise hereKoin automatically wires constructor parameters that have a definition in the graph, so annotating
dataStorewith@Provideddoes not add value. You can drop the annotation and keep the signature simpler:-@Single -internal class PrimitivePersistence(@Provided private val dataStore: DataStore<Preferences>) { +@Single +internal class PrimitivePersistence(private val dataStore: DataStore<Preferences>) {Fewer annotations → quicker reads, less KSP work.
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/loginScreen/LoginComponent.kt (1)
16-16: Injecting UseCase directly couples UI with persistence concernsThe component now depends on
SetUserLoggedInUseCase, which is fine, but it increases the surface of responsibilities inside the UI layer. Consider introducing an intermediate “LoginInteractor” (or similar) that owns the business rule and keeps the screen slimmer.shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/domain/SetUserLoggedInUseCase.kt (1)
7-13: Nit: add KDoc & considertypealias Args = BooleanFor such a small payload a full
data classis verbose. Either document the intent with KDoc or collapse it:typealias Args = BooleanNot critical but keeps the API concise.
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/root/RootNavHostComponent.kt (1)
23-26: Constructor now quite wide – consider grouping dependencies
RootNavHostComponentalready injects navigation factories, deep-link resolver and now the login UseCase. Grouping navigation concerns (e.g. aNavigationDependenciesdata holder) keeps the signature readable and eases future additions.shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/injection/PersistenceModule.kt (1)
14-21: Minor: reuse a globalJsoninstance instead of configuring on every KSP runAlthough the provider is marked
@Single, KSP will still generate a builder every compilation.
Declaring aval JsonConfig = Json { … }in an internal object avoids rebuilding the same lambda in generated code and keeps the DI graph cleaner.internal object JsonProvider { val instance: Json = Json { encodeDefaults = true isLenient = false ignoreUnknownKeys = true prettyPrint = false } } @Single @PersistenceJson internal fun provideJson(): Json = JsonProvider.instancePurely a micro-optimisation – feel free to skip if build size is not a concern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
convention-plugins/src/main/kotlin/conventions-annotations-processor.gradle.kts(2 hunks)gradle/libs.versions.toml(2 hunks)shared/app/src/commonMain/kotlin/app/futured/kmptemplate/app/injection/AppInjection.kt(2 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/domain/IsUserLoggedInUseCase.kt(1 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/domain/SetUserLoggedInUseCase.kt(1 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/root/RootNavHostComponent.kt(4 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/loginScreen/LoginComponent.kt(3 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/profileScreen/ProfileComponent.kt(3 hunks)shared/network/rest/src/commonMain/kotlin/app/futured/kmptemplate/network/rest/plugin/UserAgentPlugin.kt(1 hunks)shared/persistence/build.gradle.kts(2 hunks)shared/persistence/src/androidMain/kotlin/app/futured/kmptemplate/persistence/injection/DataStoreModule.kt(1 hunks)shared/persistence/src/androidMain/kotlin/app/futured/kmptemplate/persistence/injection/PersistencePlatformModule.kt(0 hunks)shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/injection/PersistenceModule.kt(1 hunks)shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/injection/PersistencePlatformModule.kt(0 hunks)shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/injection/Qualifiers.kt(1 hunks)shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/persistence/JsonPersistence.kt(1 hunks)shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/persistence/PrimitivePersistence.kt(1 hunks)shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/persistence/user/UserPersistence.kt(1 hunks)shared/persistence/src/iosMain/kotlin/app/futured/kmptemplate/persistence/injection/DataStoreModule.kt(1 hunks)shared/persistence/src/iosMain/kotlin/app/futured/kmptemplate/persistence/injection/PersistencePlatformModule.kt(0 hunks)
💤 Files with no reviewable changes (3)
- shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/injection/PersistencePlatformModule.kt
- shared/persistence/src/iosMain/kotlin/app/futured/kmptemplate/persistence/injection/PersistencePlatformModule.kt
- shared/persistence/src/androidMain/kotlin/app/futured/kmptemplate/persistence/injection/PersistencePlatformModule.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/profileScreen/ProfileComponent.kt (5)
shared/arkitekt-cr-usecases/src/commonMain/kotlin/app/futured/arkitekt/crusecases/scope/SingleUseCaseExecutionScope.kt (1)
onSuccess(138-140)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/profile/ProfileNavHostNavigation.kt (1)
navigateToLogin(21-21)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/root/RootNavHostNavigation.kt (1)
navigateToLogin(23-25)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/signedIn/SignedInNavHostNavigation.kt (2)
navigateToLogin(5-7)navigateToLogin(6-6)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/profileScreen/ProfileScreenNavigation.kt (2)
navigateToLogin(5-8)navigateToLogin(6-6)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check-android / Lint & Tests
- GitHub Check: check-ios / Test
🔇 Additional comments (8)
gradle/libs.versions.toml (1)
2-4: Double-check cross-version compatibilityUpgrading Kotlin (2.1.20), KSP (2.1.20-1.0.32) and Koin (4.1.0 / annotations 2.1.0) in one sweep is fine in principle, but these versions were released within days of each other. Ensure:
koin-annotations:2.1.0is actually built against Koin 4.1.x (it was for 4.0.x in previous releases).- The Compose compiler plugin you use supports Kotlin 2.1.20.
- No KSP processor (Apollo, Ktorfit, etc.) hard-pins to older Kotlin.
A dry run of
./gradlew :shared:app:assembleDebug(or CI pipeline) should reveal any ABI mismatch immediately.Also applies to: 12-14, 27-28
convention-plugins/src/main/kotlin/conventions-annotations-processor.gradle.kts (1)
22-26: Nice touch – config check enabledTurning on
KOIN_CONFIG_CHECKwill surface invalid graphs at compile-time rather than runtime. Good call.shared/network/rest/src/commonMain/kotlin/app/futured/kmptemplate/network/rest/plugin/UserAgentPlugin.kt (1)
7-11: LGTM – correct use of@ProvidedAnnotating the
platformconstructor parameter with@Providedmakes the intent explicit and relies on Koin’s generated provider. No issues spotted.shared/app/src/commonMain/kotlin/app/futured/kmptemplate/app/injection/AppInjection.kt (1)
6-7: Injection wiring updated correctlySwapping the function-based
persistenceModule()forPersistenceModule().modulealigns with the new annotation-generated modules. Looks good.Also applies to: 33-34
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/domain/IsUserLoggedInUseCase.kt (1)
7-10: Looks good – straightforward use-case wrapperThe implementation is minimal, type-safe, and follows your existing
UseCasepattern.shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/persistence/JsonPersistence.kt (1)
21-23: DI annotations look correct – good move to@Single
@ProvidedforDataStoreand qualifier forJsonaligns with the new annotation-driven DI. Nice.shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/persistence/user/UserPersistence.kt (1)
13-24: Potential IO off-main-thread guarantee
PrimitivePersistence.saveis presumably a suspend function hittingDataStore.edit, which is already dispatcher-aware, but double-check thatPrimitivePersistenceenforcesDispatchers.IO; otherwise addwithContext(Dispatchers.IO)here to avoid accidental main-thread writes.shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/injection/PersistenceModule.kt (1)
10-12: Verify Koin-KSP can resolveexpectclass inincludes
@Module(includes = [DataStoreModule::class])refers to anexpectdeclaration that is materialised only in platform source sets.
Historically, Koin-KSP struggles to resolve such symbols from common code and can silently skip the include, yielding “no definition found” at runtime.Please run the KSP task on both Android & iOS targets and ensure the generated files contain the
DataStoreModulebinding; if not, move theincludesto the platform-specificModules instead.
| import org.koin.core.annotation.Named | ||
|
|
||
| internal object Qualifiers { | ||
|
|
||
| val PersistenceJson = named("PersistenceJson") | ||
| val DataStorePath = named("DataStoreFilePath") | ||
| } | ||
| @Named | ||
| annotation class PersistenceJson |
There was a problem hiding this comment.
Wrong meta-annotation – this won’t compile
@Named requires a non-optional value argument (Named(val value: String)), so using it as a meta-annotation without a value produces a compilation error.
If you intend to create a custom qualifier, annotate it with @Qualifier instead (Koin will still recognise it), or pass an explicit value to @Named.
-import org.koin.core.annotation.Named
+import org.koin.core.annotation.Qualifier
@@
-@Named
-annotation class PersistenceJson
+@Qualifier
+annotation class PersistenceJson(Optionally add @Retention(BINARY) / @Target(*) if you need finer control.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import org.koin.core.annotation.Named | |
| internal object Qualifiers { | |
| val PersistenceJson = named("PersistenceJson") | |
| val DataStorePath = named("DataStoreFilePath") | |
| } | |
| @Named | |
| annotation class PersistenceJson | |
| import org.koin.core.annotation.Qualifier | |
| @Qualifier | |
| annotation class PersistenceJson |
🤖 Prompt for AI Agents
In
shared/persistence/src/commonMain/kotlin/app/futured/kmptemplate/persistence/injection/Qualifiers.kt
lines 3 to 6, the @Named annotation is used without the required value argument,
causing a compilation error. Replace @Named with @Qualifier to define a custom
qualifier annotation correctly, and optionally add @Retention and @Target
annotations for finer control over its usage.
| implementation(libs.koin.core) | ||
| api(libs.koin.annotations) | ||
| implementation(libs.kotlinx.coroutines.core) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid leaking koin.annotations to downstream modules
koin.annotations is only required at compile-time for KSP and is not needed on the runtime class-path of consumers. Declaring it as api will expose the dependency transitively and enlarge every consumer’s class-path.
- api(libs.koin.annotations)
+ compileOnly(libs.koin.annotations) // or `implementation` if runtime is really requiredReducing the surface keeps ABI stable and speeds up compilation of dependent modules.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| implementation(libs.koin.core) | |
| api(libs.koin.annotations) | |
| implementation(libs.kotlinx.coroutines.core) | |
| implementation(libs.koin.core) | |
| compileOnly(libs.koin.annotations) // or `implementation` if runtime is really required | |
| implementation(libs.kotlinx.coroutines.core) |
🤖 Prompt for AI Agents
In shared/persistence/build.gradle.kts around lines 33 to 35, the dependency on
koin.annotations is declared as api, which exposes it transitively to downstream
modules unnecessarily. Change the declaration from api to compileOnly to ensure
koin.annotations is only used at compile-time for KSP and not included in the
runtime class-path of consumers, reducing dependency leakage and improving build
performance.
| override fun onLogout() { | ||
| setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(false)) { | ||
| onSuccess { | ||
| navigateToLogin() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle failure path of SetUserLoggedInUseCase
Currently only the success branch is handled; if the persistence write fails the user stays on the profile screen with no feedback. At minimum capture onError and surface it (toast / snackbar) or still navigate to login if failure is non-critical.
- setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(false)) {
- onSuccess {
- navigateToLogin()
- }
- }
+ setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(false)) {
+ onSuccess { navigateToLogin() }
+ onError { /* TODO: show error or decide to navigate anyway */ }
+ }Leaving errors unhandled can hide persistence issues and confuse users.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override fun onLogout() { | |
| setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(false)) { | |
| onSuccess { | |
| navigateToLogin() | |
| } | |
| } | |
| } | |
| override fun onLogout() { | |
| setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(false)) { | |
| onSuccess { navigateToLogin() } | |
| onError { /* TODO: show error or decide to navigate anyway */ } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/profileScreen/ProfileComponent.kt
around lines 27 to 33, the onLogout function only handles the success case of
setUserLoggedInUseCase.execute and ignores failures. Add an onError handler to
capture errors from the use case execution. In the onError block, either show a
user-visible message like a toast or snackbar to indicate the failure or decide
to navigate to the login screen anyway if the failure is not critical, ensuring
the user is not left without feedback.
...ence/src/androidMain/kotlin/app/futured/kmptemplate/persistence/injection/DataStoreModule.kt
Show resolved
Hide resolved
| override fun onLoginClick() { | ||
| setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(true)) { | ||
| onSuccess { | ||
| navigateToSignedIn() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing loading / failure handling in login flow
execute { onSuccess { … } } ignores the onError and onStart / onComplete callbacks that UseCase exposes.
Users can tap the button repeatedly while the coroutine is running, or the call can fail silently and leave the UI stalled. At minimum, disable the button during the operation and surface an error if the persistence write fails.
override fun onLoginClick() {
- setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(true)) {
- onSuccess {
- navigateToSignedIn()
- }
- }
+ setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(true)) {
+ onStart { setLoading(true) }
+ onSuccess { navigateToSignedIn() }
+ onError { showError(it); setLoading(false) }
+ onComplete{ setLoading(false) }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override fun onLoginClick() { | |
| setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(true)) { | |
| onSuccess { | |
| navigateToSignedIn() | |
| } | |
| } | |
| } | |
| override fun onLoginClick() { | |
| - setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(true)) { | |
| - onSuccess { | |
| - navigateToSignedIn() | |
| - } | |
| - } | |
| + setUserLoggedInUseCase.execute(SetUserLoggedInUseCase.Args(true)) { | |
| + onStart { setLoading(true) } | |
| + onSuccess { navigateToSignedIn() } | |
| + onError { showError(it); setLoading(false) } | |
| + onComplete { setLoading(false) } | |
| + } | |
| } |
🤖 Prompt for AI Agents
In
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/loginScreen/LoginComponent.kt
between lines 28 and 34, the onLoginClick function currently only handles the
onSuccess callback of the use case execution, ignoring onError and
onStart/onComplete. To fix this, update the execute call to handle onStart by
disabling the login button, onComplete by re-enabling it, and onError by showing
an error message to the user. This will prevent multiple taps during the
operation and provide feedback if the login persistence fails.
| private fun checkUserLoggedIn() { | ||
| isUserLoggedInUseCase.execute { | ||
| onSuccess { isLoggedIn -> | ||
| if (isLoggedIn) { | ||
| rootNavigator.slotNavigator.activate(RootConfig.SignedIn()) | ||
| } else { | ||
| rootNavigator.slotNavigator.activate(RootConfig.Login) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Race-condition: deep link received after async login check
checkUserLoggedIn() is async; if a deep link arrives while the coroutine is running, the slot may first activate Login/SignedIn and then immediately re-activate the deep-link config, causing a visible “flash”.
Consider cancelling the job when a deep link is consumed or delaying slot activation until the first router decision is final.
🤖 Prompt for AI Agents
In
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/root/RootNavHostComponent.kt
around lines 92 to 102, the async checkUserLoggedIn function can cause a race
condition with deep link handling, leading to UI flashing due to multiple slot
activations. To fix this, implement cancellation of the ongoing login check
coroutine when a deep link is received or delay activating the slot navigator
until the login check completes and the first routing decision is finalized,
ensuring only one activation occurs.
| actual fun provideDataStore(): DataStore<Preferences> = PreferenceDataStoreFactory.createWithPath( | ||
| produceFile = { | ||
| val documentDirectory: NSURL? = NSFileManager.defaultManager.URLForDirectory( | ||
| directory = NSDocumentDirectory, | ||
| inDomain = NSUserDomainMask, | ||
| appropriateForURL = null, | ||
| create = false, | ||
| error = null, | ||
| ) | ||
|
|
||
| (requireNotNull(documentDirectory).path + "/$SETTINGS_DATASTORE_FILENAME").toPath() | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Handle potential null path & avoid runtime crash
NSURL.path is an optional in Apple SDKs ➜ requireNotNull(documentDirectory).path may still yield null.
Dereferencing it before the toPath() call would crash on devices where the document directory cannot be resolved (sandbox issues, corrupted container, unit-test runners, etc.).
- (requireNotNull(documentDirectory).path + "/$SETTINGS_DATASTORE_FILENAME").toPath()
+ (
+ requireNotNull(documentDirectory?.path) {
+ "Unable to resolve iOS documents directory for DataStore"
+ } + "/$SETTINGS_DATASTORE_FILENAME"
+ ).toPath()Guarding both documentDirectory and its path keeps the app from an obscure native crash and provides a clear diagnostic message instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| actual fun provideDataStore(): DataStore<Preferences> = PreferenceDataStoreFactory.createWithPath( | |
| produceFile = { | |
| val documentDirectory: NSURL? = NSFileManager.defaultManager.URLForDirectory( | |
| directory = NSDocumentDirectory, | |
| inDomain = NSUserDomainMask, | |
| appropriateForURL = null, | |
| create = false, | |
| error = null, | |
| ) | |
| (requireNotNull(documentDirectory).path + "/$SETTINGS_DATASTORE_FILENAME").toPath() | |
| }, | |
| ) | |
| actual fun provideDataStore(): DataStore<Preferences> = PreferenceDataStoreFactory.createWithPath( | |
| produceFile = { | |
| val documentDirectory: NSURL? = NSFileManager.defaultManager.URLForDirectory( | |
| directory = NSDocumentDirectory, | |
| inDomain = NSUserDomainMask, | |
| appropriateForURL = null, | |
| create = false, | |
| error = null, | |
| ) | |
| ( | |
| requireNotNull(documentDirectory?.path) { | |
| "Unable to resolve iOS documents directory for DataStore" | |
| } + "/$SETTINGS_DATASTORE_FILENAME" | |
| ).toPath() | |
| }, | |
| ) |
🤖 Prompt for AI Agents
In
shared/persistence/src/iosMain/kotlin/app/futured/kmptemplate/persistence/injection/DataStoreModule.kt
around lines 21 to 33, the code uses requireNotNull on documentDirectory but
does not check if its path property is null, which can cause a runtime crash.
Update the code to safely unwrap both documentDirectory and its path, and if
either is null, throw an exception or provide a clear error message before
calling toPath(), preventing obscure native crashes.
| @OptIn(ExperimentalForeignApi::class) | ||
| @Single | ||
| actual fun provideDataStore(): DataStore<Preferences> = PreferenceDataStoreFactory.createWithPath( | ||
| produceFile = { | ||
| val documentDirectory: NSURL? = NSFileManager.defaultManager.URLForDirectory( | ||
| directory = NSDocumentDirectory, | ||
| inDomain = NSUserDomainMask, | ||
| appropriateForURL = null, | ||
| create = false, | ||
| error = null, | ||
| ) | ||
|
|
||
| (requireNotNull(documentDirectory).path + "/$SETTINGS_DATASTORE_FILENAME").toPath() | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Provide explicit CoroutineScope to PreferenceDataStoreFactory.createWithPath
createWithPath defaults to coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.IO).
On iOS this scope lives forever, leaking after Koin.stopKoin() or during app quick-relaunches (UITests, widget processes, etc.).
Consider passing an appScope you already manage (e.g. ApplicationScope) so you can cancel it at shutdown:
-actual fun provideDataStore(): DataStore<Preferences> = PreferenceDataStoreFactory.createWithPath(
+actual fun provideDataStore(): DataStore<Preferences> = PreferenceDataStoreFactory.createWithPath(
+ coroutineScope = appScope,
produceFile = {
…
},
)If you don’t have such a scope yet, injecting one via Koin (single) keeps lifetime management transparent.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
shared/persistence/src/iosMain/kotlin/app/futured/kmptemplate/persistence/injection/DataStoreModule.kt
around lines 19 to 33, the call to PreferenceDataStoreFactory.createWithPath
does not specify a CoroutineScope, causing a scope with a SupervisorJob and
Dispatchers.IO to live indefinitely on iOS, leading to resource leaks. To fix
this, define or inject an application-wide CoroutineScope (e.g.,
ApplicationScope) and pass it explicitly as the coroutineScope parameter to
createWithPath. This allows proper cancellation of the scope during app shutdown
or Koin stop, preventing leaks.
Generated by 🚫 Danger |
Generated by 🚫 Danger |
Summary by CodeRabbit
New Features
Improvements
Dependency Updates
Refactor