KMPPT-91 Integrate SQLDlight in KMP Template#134
KMPPT-91 Integrate SQLDlight in KMP Template#134TheKalpeshPawar wants to merge 10 commits intoopenMF:devfrom
Conversation
…rt (openMF#128) - Add `KMPSQLDelightConventionPlugin` to manage SQLDelight configuration across platforms. - Implement platform-specific `SqlDriver` setups for Android, iOS (Native), Desktop (SQLite), and Web (JS/Wasm). - Introduce `MifosSQLDatabaseInitializer` to handle asynchronous schema creation. - Add `SampleRepository` and `SQLDelightSampleRepositoryImpl` to demonstrate database operations. - Update `DatabaseModule` and `DataModule` to include SQLDelight dependencies via Koin. - Configure Web workers and `sql.js` for JS/Wasm support in `cmp-web`. - Upgrade Gradle wrapper to version 9.1.0 and update project dependencies.
- Added copyright and license headers to all database module files. - Applied formatting across the module, including trailing commas and whitespace cleanup. - Standardized Koin module definitions in platform-specific SQLDelight implementations.
…tform configuration (openMF#128) * Add `SQLDelightRepositoryTest` common base class and platform-specific test implementations for Android, Desktop, Native, JS, and WasmJs. * Update `testSQLDelightPlatformModule` across all platforms to use `single` instances for `SqlDriver` and ensure proper schema initialization. * Refactor Koin module organization by moving `TestDatabaseModule` to `nonJsCommonTest` and removing the `DatabaseModule` include from `DataModule`. * Update Android and Native test modules to use `Dispatchers.IO` directly for Room query contexts. * Enhance JS/WasmJs test infrastructure with `karma.config.d` and `webpack.config.d` for `sql.js` WASM support. * Enable browser testing for JS and WasmJs in `build.gradle.kts` with automatic browser detection. * Update `libs.versions.toml` with `androidx-core` and `core-ktx` dependencies.
📝 WalkthroughWalkthroughThis PR integrates SQLDelight across the Kotlin Multiplatform project, adding convention-based plugin configuration, platform-specific SQLDelight driver implementations (Android, Desktop, Native, JS, WasmJS), a sample repository pattern, comprehensive test infrastructure, and web runtime configurations for SQL.js support with Webpack/Karma. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Koin as Koin DI
participant Module as DatabaseModule<br/>(SQLDelightModule)
participant CoroutineScope
participant Driver as SqlDriver<br/>(Platform)
participant Schema as MifosSQLDelightDatabase<br/>Schema
participant Repo as SampleRepository<br/>(Impl)
App->>Koin: Initialize App
Koin->>Module: Load DatabaseModule
Module->>CoroutineScope: Provide SupervisorJob+Dispatchers.Default
Module->>Module: Include driverModule (platform-specific)
Module->>Driver: Create platform SqlDriver<br/>(Android/Desktop/Native/JS/WasmJS)
Driver->>Schema: Initialize schema (sync/async)
Schema-->>Driver: Schema ready
Module->>Koin: Register Driver singleton
Module->>Repo: Provide MifosSQLDelightDatabase
Module->>Repo: Bind SQLDelightSampleRepositoryImpl<br/>as SampleRepository
Repo->>Koin: Register as singleton
Koin-->>App: DI graph complete
App->>Repo: Inject SampleRepository
Repo-->>App: Repository ready for queries
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (9)
core/database/build.gradle.kts-1-10 (1)
1-10:⚠️ Potential issue | 🟡 MinorRemove the duplicated MPL header.
The new 2026 block was added above the existing 2025 block, so this file now carries two license headers. Keep a single header here to avoid noisy Spotless/license-header churn.
As per coding guidelines "Apply code formatting with Spotless before committing code in Kotlin Multiplatform projects".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/build.gradle.kts` around lines 1 - 10, This file currently contains a duplicated MPL license header (the new 2026 block added above an existing 2025 block); remove the older/duplicate header and keep a single license header (prefer the updated 2026 block) so the file has only one comment header at the top, then run Spotless formatting to normalize the file; look for the top comment blocks starting with "Copyright 2026" and the older "Copyright 2025" and delete the redundant one.core/database/src/androidUnitTest/kotlin/org/mifos/core/database/SQLDelightRepositoryAndroidTest.kt-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorMinor typo in license header: duplicate "See".
- * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE + * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/androidUnitTest/kotlin/org/mifos/core/database/SQLDelightRepositoryAndroidTest.kt` at line 8, The license header contains a duplicated word "See See" — update the header text to remove the duplicate so it reads "See https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; this change is in the file near the top of the test class SQLDelightRepositoryAndroidTest.kt (the license header comment above the class).core/database/src/nativeTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.native.kt-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorMinor typo in license header: duplicate "See".
- * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE + * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/nativeTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.native.kt` at line 8, Fix the duplicate word in the file's license header by removing the extra "See" so the header reads "See https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; update the top-of-file license comment in TestSQLDelightModule.native.kt (the header comment block) to eliminate the repeated "See".core/database/src/wasmJsTest/kotlin/org/mifos/core/database/SQLDelightRepositoryWasmJsTest.kt-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorMinor typo in license header: duplicate "See".
- * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE + * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/wasmJsTest/kotlin/org/mifos/core/database/SQLDelightRepositoryWasmJsTest.kt` at line 8, The license header contains a duplicated word "See See" at the top of SQLDelightRepositoryWasmJsTest.kt; edit the file's header comment to remove the extra "See" so it reads "See https://github.com/openMF/kmp-project-template/blob/main/LICENSE" (locate the header comment in SQLDelightRepositoryWasmJsTest.kt and correct the duplicated word).core/database/src/nativeMain/kotlin/org/mifos/core/database/di/SQLDelightModule.native.kt-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorMinor typo in license header: "See See" should be "See".
The license header contains a duplicate word.
- * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE + * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/nativeMain/kotlin/org/mifos/core/database/di/SQLDelightModule.native.kt` at line 8, Fix the license header typo by removing the duplicate word so the header reads "See https://github.com/openMF/kmp-project-template/blob/main/LICENSE" instead of "See See ..."; update the header comment in SQLDelightModule.native.kt (the top-of-file license comment) to replace "See See" with a single "See".core/database/src/androidMain/kotlin/org/mifos/core/database/di/SQLDelightModule.android.kt-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorMinor typo in license header: duplicate "See".
- * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE + * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/androidMain/kotlin/org/mifos/core/database/di/SQLDelightModule.android.kt` at line 8, The license header contains a duplicated word "See See" — update the header by removing the extra "See" so it reads "See https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; locate the string "See See" in the file (core/database/src/androidMain/kotlin/org/mifos/core/database/di/SQLDelightModule.android.kt) and replace it with the corrected single "See".core/database/src/desktopTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.desktop.kt-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorMinor typo in license header: duplicate "See".
- * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE + * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/desktopTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.desktop.kt` at line 8, The license header contains a duplicated word "See See" — remove the extra "See" so the header reads "See https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; update the header comment in TestSQLDelightModule.desktop.kt (look for the string "See See") to fix the typo.core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.wasmJs.kt-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorMinor typo in license header: duplicate "See".
- * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE + * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.wasmJs.kt` at line 8, The license header contains a duplicated word "See See"—remove the extra "See" so the header reads "See https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; update the header comment in SQLDelightModule.wasmJs.kt (the top-of-file license header) to remove the duplicate word while preserving the rest of the license text and link.core/database/src/commonTest/kotlin/org/mifos/core/database/SQLDelightRepositoryTest.kt-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorMinor typo: Duplicate "See" in license header.
- * See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE + * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/commonTest/kotlin/org/mifos/core/database/SQLDelightRepositoryTest.kt` at line 8, The license header contains a duplicated word "See See" in the header comment; edit the header comment (the license header line containing "See See https://github.com/openMF/kmp-project-template/blob/main/LICENSE") and remove the extra "See" so it reads "See https://github.com/openMF/kmp-project-template/blob/main/LICENSE".
🧹 Nitpick comments (4)
core/database/src/commonMain/sqldelight/org/mifos/core/database/Sample.sq (1)
9-10:OR REPLACEdoes not buy anything here.
idis autogenerated andnameis not unique, so this currently behaves like a plain insert. KeepingREPLACEis misleading, and if a unique constraint is added later it will switch to delete+insert semantics that can unexpectedly change row ids.Suggested fix
insertSample: -INSERT OR REPLACE INTO sample (name) VALUES (:name); +INSERT INTO sample (name) VALUES (:name);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/commonMain/sqldelight/org/mifos/core/database/Sample.sq` around lines 9 - 10, The SQL statement for insertSample should not use "OR REPLACE" since id is autogenerated and name is not unique; update the insertSample query to a plain INSERT INTO sample (name) VALUES (:name) (remove "OR REPLACE") so it performs a normal insert into the sample table for the name parameter.core/database/src/androidUnitTest/kotlin/org/mifos/core/database/SQLDelightRepositoryAndroidTest.kt (1)
35-38: Redundant override:teardown()only delegates to super.This override can be removed since it adds no platform-specific cleanup logic.
♻️ Suggested removal
- `@AfterTest` - override fun teardown() { - super.teardown() - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/androidUnitTest/kotlin/org/mifos/core/database/SQLDelightRepositoryAndroidTest.kt` around lines 35 - 38, Remove the redundant `@AfterTest` override teardown() in SQLDelightRepositoryAndroidTest (the method that only calls super.teardown()), since it adds no platform-specific cleanup; delete the entire override (annotation and method) so the superclass implementation is used directly and no-op delegation remains.core/database/src/commonTest/kotlin/org/mifos/core/database/SQLDelightRepositoryTest.kt (1)
80-99: Consider simplifying the delete test by extracting ID retrieval.The current pattern of capturing
aliceIdvia reassignment inside a Turbine block is functional but somewhat awkward. Consider extracting the ID retrieval logic for clarity.♻️ Suggested refactor using a helper function
+ private suspend fun getIdByName(name: String): Long { + var id = 0L + repository.getAllSamples().test { + id = awaitItem().first { it.name == name }.id + cancelAndIgnoreRemainingEvents() + } + return id + } + `@Test` fun deleteSampleRemovesItFromDatabase() = runTest { repository.insertSample("Alice") repository.insertSample("Bob") - var aliceId = 0L - repository.getAllSamples().test { - aliceId = awaitItem().first { it.name == "Alice" }.id - cancelAndIgnoreRemainingEvents() - } + val aliceId = getIdByName("Alice") repository.deleteById(aliceId) repository.getAllSamples().test { val remaining = awaitItem() assertEquals(1, remaining.size) assertEquals("Bob", remaining[0].name) cancelAndIgnoreRemainingEvents() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/commonTest/kotlin/org/mifos/core/database/SQLDelightRepositoryTest.kt` around lines 80 - 99, Refactor the test deleteSampleRemovesItFromDatabase by extracting the logic that retrieves Alice's id from the flow into a small helper used by the test (e.g., a suspend function like getIdByName(name: String) that collects repository.getAllSamples(), awaits the first emitted list, finds the item with the given name and returns its id); then call this helper to obtain aliceId before calling repository.deleteById(aliceId), keeping the assertions in the Turbine blocks unchanged for clarity and reuse.core/database/src/webpack.config.d/sqljs-config.js (1)
9-15: Resolve the wasm asset via module resolution.
CopyWebpackPluginresolves relativefromvalues from webpack'scontext, not from this snippet file, so'../../node_modules/...'is tightly coupled to the generated build layout. Usingrequire.resolve("sql.js/dist/sql-wasm.wasm")is more stable across Gradle/Kotlin JS output locations. (webpack.js.org)Suggested refactor
const CopyWebpackPlugin = require('copy-webpack-plugin'); config.plugins.push( new CopyWebpackPlugin({ patterns: [ - '../../node_modules/sql.js/dist/sql-wasm.wasm' + require.resolve("sql.js/dist/sql-wasm.wasm") ] }) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/webpack.config.d/sqljs-config.js` around lines 9 - 15, In the CopyWebpackPlugin patterns array, replace the hardcoded relative path string `'../../node_modules/sql.js/dist/sql-wasm.wasm'` with a dynamic module resolution using require.resolve() to resolve the sql.js package and its wasm file path. This will decouple the configuration from the specific build layout and make it more robust across different Gradle/Kotlin JS output locations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build-logic/convention/src/main/kotlin/KMPSQLDelightConventionPlugin.kt`:
- Around line 17-25: The SQLDelight configuration currently disables migration
verification in the SqlDelightExtension block created via
databases.create(DATABASE_NAME) by setting verifyMigrations.set(false); update
this to enable verification (e.g., set verifyMigrations.set(true) or remove the
explicit false override) so modules using the KMPSQLDelightConventionPlugin
inherit migration checks; ensure the change is made in the
configure<SqlDelightExtension> -> databases.create(DATABASE_NAME) section where
packageName, generateAsync and schemaOutputDirectory are set.
In `@core/database/build.gradle.kts`:
- Around line 44-52: The CHROME_BIN environment is only set when
isInstalled("brave") is true, missing cases where the binary is named
"brave-browser"; update both testTask blocks that currently call
environment("CHROME_BIN", "brave") to handle either binary: check
isInstalled("brave") || isInstalled("brave-browser") and set CHROME_BIN to
"brave" if isInstalled("brave") otherwise "brave-browser" (use the same logic in
both places where environment("CHROME_BIN", "brave") is currently set).
- Around line 98-103: The build uses a non-existent source set jsCommonMain so
the npm dependencies aren’t attached to any target; either move the dependency
blocks into the actual JS targets by adding the npm/sql deps to
jsMain.dependencies and wasmJsMain.dependencies, or explicitly create a
jsCommonMain source set (val jsCommonMain by creating {
dependsOn(commonMain.get()) }) and wire it into the hierarchy so that
applyProjectHierarchyTemplate() will propagate those dependencies; update the
blocks referencing implementation(npm(...))/implementation(devNpm(...))
accordingly to ensure the npm packages are available at runtime.
In `@core/database/karma.config.d/sqljs-config.js`:
- Around line 3-15: The wasm path resolution and proxy URL construction are
fragile: replace the manual path.resolve logic with require.resolve to locate
the wasm file (use require.resolve("sql.js/dist/sql-wasm.wasm") to set wasm),
ensure the filesystem path is converted to a URL-friendly form by
normalizing/backslash-replacing (e.g., replace backslashes with forward slashes)
before building the proxy string, and guard access to config.proxies (create it
if falsy) before assigning config.proxies["/sql-wasm.wasm"] =
`/absolute${normalizedWasmPath}`; update references to dist, wasm, config.files
and config.proxies accordingly.
In
`@core/database/src/commonMain/kotlin/org/mifos/core/database/di/MifosSQLDatabaseInitializer.kt`:
- Around line 22-25: The initialize() function currently launches schema
creation asynchronously (scope.launch {
MifosSQLDelightDatabase.Schema.awaitCreate(driver) }) which returns immediately
and can allow DB access before the schema exists; change initialize() to perform
schema creation synchronously or provide an awaitable completion: either make
initialize() a suspend function and call
MifosSQLDelightDatabase.Schema.awaitCreate(driver) directly, or have
initialize() return the launching Job/Deferred so callers can await it; update
any callers and DI wiring to call/await the new suspend initialize() (or await
the returned Job) instead of relying on scope.launch.
In
`@core/database/src/commonMain/kotlin/org/mifos/core/database/di/SQLDelightModule.kt`:
- Around line 23-31: The DI currently starts MifosSQLDatabaseInitializer
asynchronously (MifosSQLDatabaseInitializer.initialize() launches a coroutine)
but then immediately registers MifosSQLDelightDatabase, causing a race; change
the registration so initialization completes before creating
MifosSQLDelightDatabase: make the initializer expose a suspend/blocking init (or
call initialize() inside runBlocking) in the single(createdAtStart = true) block
and only after that return/create MifosSQLDelightDatabase, or else make the
MifosSQLDelightDatabase single depend on a synchronous result from
MifosSQLDatabaseInitializer.initialize() so Koin blocks until schema creation
finishes before resolving MifosSQLDelightDatabase.
In `@core/database/src/commonMain/sqldelight/org/mifos/core/database/Sample.sq`:
- Around line 6-7: The selectAll query in Sample.sq returns rows without a
stable order; update the selectAll SQL to include an explicit ORDER BY using a
stable column (e.g., the primary key or a timestamp) so results are
deterministic across drivers — for example change the selectAll query in
Sample.sq to SELECT * FROM sample ORDER BY <primary_key_or_created_at> ASC (use
the actual PK column name or rowid if no explicit PK).
In
`@core/database/src/desktopMain/kotlin/org/mifos/core/database/di/SQLDelightModule.desktop.kt`:
- Around line 21-24: The desktop driver currently calls
MifosSQLDelightDatabase.Schema.create(...) during JdbcSqliteDriver instantiation
which is non-idempotent and will fail if the DB file exists; remove that call
and either wrap the schema with MifosSQLDelightDatabase.Schema.synchronous(...)
when passing it to the driver (matching Android/Native), or omit schema creation
here entirely and let MifosSQLDatabaseInitializer perform initialization via its
coroutine flow; update the code around JdbcSqliteDriver(...) to stop invoking
Schema.create and use Schema.synchronous or defer to MifosSQLDatabaseInitializer
accordingly.
In
`@core/database/src/jsTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.js.kt`:
- Around line 21-26: The JS test module returns the SqlDriver before schema
creation completes because schema setup runs asynchronously in
MainScope().launch; change the initialization inside the single<SqlDriver>
provider to perform schema creation synchronously by using runBlocking and
awaiting MifosSQLDelightDatabase.Schema.awaitCreate(driver) before returning the
driver (replace the MainScope().launch block with runBlocking around the await
call for createDefaultWebWorkerDriver()), ensuring the driver is only returned
after the schema is created.
In
`@core/database/src/nativeTest/kotlin/org/mifos/core/database/di/TestDatabaseModule.native.kt`:
- Around line 14-15: Remove the invalid top-level import kotlinx.coroutines.IO
and instead reference Dispatchers.IO directly where used (keep the existing
import kotlinx.coroutines.Dispatchers), and remove the unnecessary explicit cast
to CoroutineContext so code uses Dispatchers.IO as a CoroutineContext without
casting (look for usages in TestDatabaseModule.native.kt referencing
Dispatchers, IO, and CoroutineContext).
In
`@core/database/src/wasmJsTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.wasmJs.kt`:
- Around line 21-26: The driver is returned before schema creation completes
because createDefaultWebWorkerDriver() uses MainScope().launch; replace the
detached coroutine with a blocking wait so initialization finishes before
returning: in the single<SqlDriver> provider that calls
createDefaultWebWorkerDriver(), invoke runBlocking {
MifosSQLDelightDatabase.Schema.awaitCreate(driver) } (using the returned driver
variable) instead of MainScope().launch, ensuring the test driver is fully
initialized before being returned.
In `@core/database/src/webpack.config.d/sqljs-config.js`:
- Around line 1-7: The current assignment replaces the entire config.resolve
object and loses defaults; instead merge the fallback into the existing resolve
object by updating config.resolve rather than overwriting it (e.g., set
config.resolve = { ...config.resolve, fallback: { fs: false, path: false,
crypto: false } } or assign to config.resolve.fallback), ensuring existing
resolve properties (like extensions and modules) are preserved while adding the
fallback entries.
---
Minor comments:
In `@core/database/build.gradle.kts`:
- Around line 1-10: This file currently contains a duplicated MPL license header
(the new 2026 block added above an existing 2025 block); remove the
older/duplicate header and keep a single license header (prefer the updated 2026
block) so the file has only one comment header at the top, then run Spotless
formatting to normalize the file; look for the top comment blocks starting with
"Copyright 2026" and the older "Copyright 2025" and delete the redundant one.
In
`@core/database/src/androidMain/kotlin/org/mifos/core/database/di/SQLDelightModule.android.kt`:
- Line 8: The license header contains a duplicated word "See See" — update the
header by removing the extra "See" so it reads "See
https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; locate the
string "See See" in the file
(core/database/src/androidMain/kotlin/org/mifos/core/database/di/SQLDelightModule.android.kt)
and replace it with the corrected single "See".
In
`@core/database/src/androidUnitTest/kotlin/org/mifos/core/database/SQLDelightRepositoryAndroidTest.kt`:
- Line 8: The license header contains a duplicated word "See See" — update the
header text to remove the duplicate so it reads "See
https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; this change
is in the file near the top of the test class SQLDelightRepositoryAndroidTest.kt
(the license header comment above the class).
In
`@core/database/src/commonTest/kotlin/org/mifos/core/database/SQLDelightRepositoryTest.kt`:
- Line 8: The license header contains a duplicated word "See See" in the header
comment; edit the header comment (the license header line containing "See See
https://github.com/openMF/kmp-project-template/blob/main/LICENSE") and remove
the extra "See" so it reads "See
https://github.com/openMF/kmp-project-template/blob/main/LICENSE".
In
`@core/database/src/desktopTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.desktop.kt`:
- Line 8: The license header contains a duplicated word "See See" — remove the
extra "See" so the header reads "See
https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; update the
header comment in TestSQLDelightModule.desktop.kt (look for the string "See
See") to fix the typo.
In
`@core/database/src/nativeMain/kotlin/org/mifos/core/database/di/SQLDelightModule.native.kt`:
- Line 8: Fix the license header typo by removing the duplicate word so the
header reads "See
https://github.com/openMF/kmp-project-template/blob/main/LICENSE" instead of
"See See ..."; update the header comment in SQLDelightModule.native.kt (the
top-of-file license comment) to replace "See See" with a single "See".
In
`@core/database/src/nativeTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.native.kt`:
- Line 8: Fix the duplicate word in the file's license header by removing the
extra "See" so the header reads "See
https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; update the
top-of-file license comment in TestSQLDelightModule.native.kt (the header
comment block) to eliminate the repeated "See".
In
`@core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.wasmJs.kt`:
- Line 8: The license header contains a duplicated word "See See"—remove the
extra "See" so the header reads "See
https://github.com/openMF/kmp-project-template/blob/main/LICENSE"; update the
header comment in SQLDelightModule.wasmJs.kt (the top-of-file license header) to
remove the duplicate word while preserving the rest of the license text and
link.
In
`@core/database/src/wasmJsTest/kotlin/org/mifos/core/database/SQLDelightRepositoryWasmJsTest.kt`:
- Line 8: The license header contains a duplicated word "See See" at the top of
SQLDelightRepositoryWasmJsTest.kt; edit the file's header comment to remove the
extra "See" so it reads "See
https://github.com/openMF/kmp-project-template/blob/main/LICENSE" (locate the
header comment in SQLDelightRepositoryWasmJsTest.kt and correct the duplicated
word).
---
Nitpick comments:
In
`@core/database/src/androidUnitTest/kotlin/org/mifos/core/database/SQLDelightRepositoryAndroidTest.kt`:
- Around line 35-38: Remove the redundant `@AfterTest` override teardown() in
SQLDelightRepositoryAndroidTest (the method that only calls super.teardown()),
since it adds no platform-specific cleanup; delete the entire override
(annotation and method) so the superclass implementation is used directly and
no-op delegation remains.
In `@core/database/src/commonMain/sqldelight/org/mifos/core/database/Sample.sq`:
- Around line 9-10: The SQL statement for insertSample should not use "OR
REPLACE" since id is autogenerated and name is not unique; update the
insertSample query to a plain INSERT INTO sample (name) VALUES (:name) (remove
"OR REPLACE") so it performs a normal insert into the sample table for the name
parameter.
In
`@core/database/src/commonTest/kotlin/org/mifos/core/database/SQLDelightRepositoryTest.kt`:
- Around line 80-99: Refactor the test deleteSampleRemovesItFromDatabase by
extracting the logic that retrieves Alice's id from the flow into a small helper
used by the test (e.g., a suspend function like getIdByName(name: String) that
collects repository.getAllSamples(), awaits the first emitted list, finds the
item with the given name and returns its id); then call this helper to obtain
aliceId before calling repository.deleteById(aliceId), keeping the assertions in
the Turbine blocks unchanged for clarity and reuse.
In `@core/database/src/webpack.config.d/sqljs-config.js`:
- Around line 9-15: In the CopyWebpackPlugin patterns array, replace the
hardcoded relative path string `'../../node_modules/sql.js/dist/sql-wasm.wasm'`
with a dynamic module resolution using require.resolve() to resolve the sql.js
package and its wasm file path. This will decouple the configuration from the
specific build layout and make it more robust across different Gradle/Kotlin JS
output locations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c72371c2-a1ea-4c50-b5df-f70c23b9582d
⛔ Files ignored due to path filters (2)
kotlin-js-store/wasm/yarn.lockis excluded by!**/yarn.lock,!**/*.lockkotlin-js-store/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (42)
build-logic/convention/build.gradle.ktsbuild-logic/convention/src/main/kotlin/KMPCoreBaseLibraryConventionPlugin.ktbuild-logic/convention/src/main/kotlin/KMPSQLDelightConventionPlugin.ktbuild-logic/convention/src/main/kotlin/org/convention/KotlinMultiplatform.ktbuild.gradle.ktscmp-shared/cmp_shared.podspeccmp-web/karma.config.d/sqljs.jscmp-web/webpack.config.d/sqljs.jscore-base/database/build.gradle.ktscore/database/build.gradle.ktscore/database/karma.config.d/sqljs-config.jscore/database/src/androidMain/kotlin/org/mifos/core/database/di/SQLDelightModule.android.ktcore/database/src/androidUnitTest/kotlin/org/mifos/core/database/SQLDelightRepositoryAndroidTest.ktcore/database/src/androidUnitTest/kotlin/org/mifos/core/database/di/TestDatabaseModule.android.ktcore/database/src/androidUnitTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.android.ktcore/database/src/commonMain/kotlin/org/mifos/core/database/di/DatabaseModule.ktcore/database/src/commonMain/kotlin/org/mifos/core/database/di/MifosSQLDatabaseInitializer.ktcore/database/src/commonMain/kotlin/org/mifos/core/database/di/SQLDelightModule.ktcore/database/src/commonMain/kotlin/org/mifos/core/database/repository/SQLDelightSampleRepositoryImpl.ktcore/database/src/commonMain/kotlin/org/mifos/core/database/repository/SampleRepository.ktcore/database/src/commonMain/sqldelight/org/mifos/core/database/Sample.sqcore/database/src/commonTest/kotlin/org/mifos/core/database/SQLDelightRepositoryTest.ktcore/database/src/commonTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.ktcore/database/src/desktopMain/kotlin/org/mifos/core/database/di/SQLDelightModule.desktop.ktcore/database/src/desktopTest/kotlin/org/mifos/core/database/SQLDelightRepositoryDesktopTest.ktcore/database/src/desktopTest/kotlin/org/mifos/core/database/di/TestDatabaseModule.desktop.ktcore/database/src/desktopTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.desktop.ktcore/database/src/jsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.js.ktcore/database/src/jsTest/kotlin/org/mifos/core/database/SQLDelightRepositoryJsTest.ktcore/database/src/jsTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.js.ktcore/database/src/nativeMain/kotlin/org/mifos/core/database/di/DatabaseModule.native.ktcore/database/src/nativeMain/kotlin/org/mifos/core/database/di/SQLDelightModule.native.ktcore/database/src/nativeTest/kotlin/org/mifos/core/database/SQLDelightRepositoryNativeTest.ktcore/database/src/nativeTest/kotlin/org/mifos/core/database/di/TestDatabaseModule.native.ktcore/database/src/nativeTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.native.ktcore/database/src/nonJsCommonTest/kotlin/org/mifos/core/database/di/TestDatabaseModule.ktcore/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.wasmJs.ktcore/database/src/wasmJsTest/kotlin/org/mifos/core/database/SQLDelightRepositoryWasmJsTest.ktcore/database/src/wasmJsTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.wasmJs.ktcore/database/src/webpack.config.d/sqljs-config.jsgradle/libs.versions.tomlgradle/wrapper/gradle-wrapper.properties
💤 Files with no reviewable changes (1)
- build-logic/convention/src/main/kotlin/KMPCoreBaseLibraryConventionPlugin.kt
core/database/src/commonMain/kotlin/org/mifos/core/database/di/MifosSQLDatabaseInitializer.kt
Outdated
Show resolved
Hide resolved
core/database/src/desktopMain/kotlin/org/mifos/core/database/di/SQLDelightModule.desktop.kt
Outdated
Show resolved
Hide resolved
core/database/src/jsTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.js.kt
Show resolved
Hide resolved
core/database/src/nativeTest/kotlin/org/mifos/core/database/di/TestDatabaseModule.native.kt
Outdated
Show resolved
Hide resolved
core/database/src/wasmJsTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.wasmJs.kt
Show resolved
Hide resolved
| config.resolve = { | ||
| fallback: { | ||
| fs: false, | ||
| path: false, | ||
| crypto: false, | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "sqljs-config.js" | head -20Repository: openMF/kmp-project-template
Length of output: 170
🏁 Script executed:
fd -name "sqljs-config.js" -type fRepository: openMF/kmp-project-template
Length of output: 301
🌐 Web query:
webpack resolve configuration merge behavior replace vs merge
💡 Result:
When you “merge” webpack configs, the behavior depends on what tool is doing the merge:
webpack-cli --mergeuseswebpack-merge(it’s not webpack core doing it). [1]webpack-mergedefault behavior: it merges objects and concatenates arrays (and for the same scalar key, the value “to the right” wins). Soresolve(object) is merged, whileresolve.extensions,resolve.modules, etc. (arrays) are concatenated by default. [2]
How “replace vs merge” works for resolve
resolve.alias(object): merged by key; if both configs define the same alias key, the later config’s value wins (normal “right wins” key precedence). [2]resolve.extensions(array): concatenated by default inwebpack-merge, so you can accidentally end up with combined/duplicate extensions unless you customize. [2]
How to force “replace” (instead of concat/merge)
Use mergeWithCustomize + customizeArray/customizeObject and set specific fields to replace (it supports append | prepend | replace, including wildcard paths). [2]
Related gotcha (not about merging, but often confused with it)
In plain webpack config (no merging), setting resolve.extensions overrides webpack’s default extensions list—to keep defaults and add more, use '...'. [3]
Sources: [1] webpack CLI docs (--merge uses webpack-merge), [2] webpack-merge README (default concat/merge + replace strategies), [3] webpack resolve.extensions docs (override defaults + '...').
🏁 Script executed:
cat -n ./core/database/src/webpack.config.d/sqljs-config.jsRepository: openMF/kmp-project-template
Length of output: 496
🏁 Script executed:
find . -name "webpack.config.js" -o -name "webpack*.config.js" | grep -v node_modules | head -20Repository: openMF/kmp-project-template
Length of output: 53
🏁 Script executed:
rg "sqljs-config" --type jsRepository: openMF/kmp-project-template
Length of output: 53
🏁 Script executed:
find . -path "*/webpack.config.d/*" -type fRepository: openMF/kmp-project-template
Length of output: 210
🏁 Script executed:
rg "webpack.config.d" --type js --type jsonRepository: openMF/kmp-project-template
Length of output: 53
🏁 Script executed:
rg "webpack\.config\.d" --type jsRepository: openMF/kmp-project-template
Length of output: 53
🏁 Script executed:
find . -maxdepth 3 -name "webpack.config.js" -o -name "webpack.*.js" | grep -v node_modulesRepository: openMF/kmp-project-template
Length of output: 53
🏁 Script executed:
rg "require.*webpack" --type js | grep -i config | head -20Repository: openMF/kmp-project-template
Length of output: 361
🏁 Script executed:
find . -maxdepth 2 -name "package.json" -exec grep -l "webpack" {} \;Repository: openMF/kmp-project-template
Length of output: 53
🏁 Script executed:
cat ./package.json | head -50Repository: openMF/kmp-project-template
Length of output: 117
🏁 Script executed:
rg "webpack" package.jsonRepository: openMF/kmp-project-template
Length of output: 123
🏁 Script executed:
ls -la | head -20Repository: openMF/kmp-project-template
Length of output: 1254
🏁 Script executed:
find . -maxdepth 2 -name "build.gradle*" -o -name "settings.gradle*"Repository: openMF/kmp-project-template
Length of output: 298
🏁 Script executed:
find . -maxdepth 3 -name "package.json"Repository: openMF/kmp-project-template
Length of output: 53
🏁 Script executed:
find cmp-web -name "webpack.config.js" -o -name "webpack*.js" | head -20Repository: openMF/kmp-project-template
Length of output: 53
🏁 Script executed:
find core/database -name "webpack.config.js" -o -name "webpack*.js" | head -20Repository: openMF/kmp-project-template
Length of output: 53
🏁 Script executed:
cat cmp-web/build.gradle.kts | head -100Repository: openMF/kmp-project-template
Length of output: 1916
🌐 Web query:
Kotlin/JS webpack.config.d configuration merge behavior how files loaded
💡 Result:
In Kotlin/JS, webpack.config.d is not a “webpack-merge” (object/deep-merge) mechanism. The Kotlin Gradle plugin generates build/js/packages/<projectName>/webpack.config.js and then appends the contents of your extra config scripts into that generated file, where they run as plain JS that mutates a global config object. So “merge” here effectively means “concatenate and execute”. [1][2]
Which files are loaded
- The plugin takes the configured
configDirectory(default is the project-rootwebpack.config.d) and includes only files ending in.js. [1][2][3]
When/where they are applied
- The appended snippets are placed near the end of the generated config (before
module.exports = config), so they can override earlier defaults. [2]
Order / precedence
- In the implementation (at least in the Kotlin Gradle plugin source shown), the directory files are iterated via
confDir.listFiles()and then filtered to.js—no sorting is applied, so the effective order is filesystem-dependent and not guaranteed. If multiple files set the same field (for exampleconfig.devServer = ...), the one that runs last wins. [3]
Practical guidance
- If ordering matters, prefer one config file, or ensure deterministic behavior by avoiding “replace” assignments and instead mutating/merging (for example pushing to arrays like
config.module.rules.push(...), or doingObject.assigninto an existing object). [1][2]
Sources
- Kotlin docs: extra
.jsfiles inwebpack.config.dare “merged” into the generatedwebpack.config.jsand should modify the globalconfig. [1] - Kotlin Gradle plugin source (
KotlinWebpackConfig.appendFromConfigDir()callsappendConfigsFromDir(...)before exporting config). [2] - Kotlin Gradle plugin source (
appendConfigsFromDirusesconfDir.listFiles()then filters.jsand appends each file’s text). [3]
[1] (kotlinlang.org)
[2] (github.com)
[3] (github.com)
Citations:
- 1: https://kotlinlang.org/docs/js-project-setup.html
- 2: https://github.com/JetBrains/kotlin/blob/3e251668324f3ac2f0819ca6bef0ce97ca26f39b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/js/webpack/KotlinWebpackConfig.kt
- 3: https://github.com/JetBrains/kotlin/blob/3e251668324f3ac2f0819ca6bef0ce97ca26f39b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/js/utils.kt
🌐 Web query:
Kotlin Multiplatform JavaScript webpack config.d fragments merge
💡 Result:
In Kotlin Multiplatform/JS, webpack.config.d isn’t merged via a webpack-style deep merge. Instead, during the webpack tasks, all .js files from <projectRoot>/webpack.config.d are injected into the generated build/js/packages/<projectName>/webpack.config.js (effectively appended to the generated config file) and executed in that file’s context. [1], [2]
Practical implications:
- Write fragments to mutate the provided
configobject (for example:config.module.rules.push(...),config.plugins.push(...), etc.). [1] - Don’t rely on changing
module.exports(for example, exporting a function) inside a fragment: the Kotlin-generated config ends withmodule.exports = config, which can override what your fragment exports. [3] - If multiple fragments touch the same fields, later executed statements win (because it’s just sequential JS in one file). [2]
Sources:
[1] Kotlin docs – “Set up a Kotlin/JS project” (webpack.config.d files are “merged” into the generated webpack.config.js) (kotlinlang.org)
[2] Kotlin Slack archive – “All js files in there are copied at the bottom of generated config file” (slack-chats.kotlinlang.org)
[3] Kotlin Discussions – example showing fragment code added, then overwritten by trailing module.exports = config (discuss.kotlinlang.org)
Citations:
- 1: https://kotlinlang.org/docs/js-project-setup.html
- 2: https://slack-chats.kotlinlang.org/t/466173/hello-how-can-i-configure-kotlin-js-so-that-in-webpack-confi
- 3: https://discuss.kotlinlang.org/t/setting-environment-variable-with-kotlinjs-and-webpack-5/26777
Merge into config.resolve; don't replace it.
The Kotlin/JS Gradle plugin generates a webpack config with default resolve settings (such as resolve.extensions and resolve.modules). Direct assignment on line 1 overwrites this entire object, discarding those defaults. Merge the fallback setting into the existing resolve object instead.
Suggested fix
-config.resolve = {
- fallback: {
- fs: false,
- path: false,
- crypto: false,
- }
-};
+config.resolve = {
+ ...(config.resolve ?? {}),
+ fallback: {
+ ...(config.resolve?.fallback ?? {}),
+ fs: false,
+ path: false,
+ crypto: false,
+ },
+};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/database/src/webpack.config.d/sqljs-config.js` around lines 1 - 7, The
current assignment replaces the entire config.resolve object and loses defaults;
instead merge the fallback into the existing resolve object by updating
config.resolve rather than overwriting it (e.g., set config.resolve = {
...config.resolve, fallback: { fs: false, path: false, crypto: false } } or
assign to config.resolve.fallback), ensuring existing resolve properties (like
extensions and modules) are preserved while adding the fallback entries.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
build-logic/convention/src/main/kotlin/KMPSQLDelightConventionPlugin.kt (1)
5-15: Avoidorg.gradle.internalin this convention plugin.
org.gradle.internal.Actions.withis not public Gradle API, so this build logic becomes unnecessarily brittle on Gradle upgrades. Kotlin'swith(target)ortarget.run { ... }is enough here.♻️ Proposed change
-import org.gradle.internal.Actions.with @@ class KMPSQLDelightConventionPlugin : Plugin<Project> { override fun apply(target: Project) { - with(target) { + target.run { pluginManager.apply("app.cash.sqldelight")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-logic/convention/src/main/kotlin/KMPSQLDelightConventionPlugin.kt` around lines 5 - 15, Remove the internal Gradle import and usage of org.gradle.internal.Actions.with in KMPSQLDelightConventionPlugin; delete the import line and change the apply method to use Kotlin's built-in scope function (e.g., with(target) or target.run { ... }) so the plugin uses only public APIs—update references around the apply method and keep DATABASE_NAME and pluginManager.apply("app.cash.sqldelight") intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@build-logic/convention/src/main/kotlin/KMPSQLDelightConventionPlugin.kt`:
- Around line 5-15: Remove the internal Gradle import and usage of
org.gradle.internal.Actions.with in KMPSQLDelightConventionPlugin; delete the
import line and change the apply method to use Kotlin's built-in scope function
(e.g., with(target) or target.run { ... }) so the plugin uses only public
APIs—update references around the apply method and keep DATABASE_NAME and
pluginManager.apply("app.cash.sqldelight") intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec14f976-4332-481e-8204-2f7e011d0eca
📒 Files selected for processing (1)
build-logic/convention/src/main/kotlin/KMPSQLDelightConventionPlugin.kt
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/database/build.gradle.kts (1)
1-20:⚠️ Potential issue | 🟡 MinorRemove duplicate license header.
The file contains two license headers: one dated 2026 (lines 1-9) and another dated 2025 (lines 12-20). Only one header should be present.
Proposed fix
/* * Copyright 2026 Mifos Initiative * * This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. * * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE */ import org.gradle.kotlin.dsl.invoke -/* - * Copyright 2025 Mifos Initiative - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at https://mozilla.org/MPL/2.0/. - * - * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE - */ plugins {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/build.gradle.kts` around lines 1 - 20, Remove the duplicated license header by keeping only a single header block; locate the two consecutive license comment blocks (one beginning "Copyright 2026 Mifos Initiative" and the other "Copyright 2025 Mifos Initiative") at the top of core/database/build.gradle.kts and delete the redundant one so only one correct license header remains.
🧹 Nitpick comments (1)
core/database/build.gradle.kts (1)
35-40: Consider closing Process streams to avoid potential resource leaks.
Runtime.exec()returns aProcesswhose input/output streams should ideally be consumed or closed. In a build script context this is typically benign, but for robustness you could useProcessBuilderwith inherited I/O or explicitly close streams.Optional improvement
fun isInstalled(binary: String) = try { val command = if (isWindows) arrayOf("where", binary) else arrayOf("which", binary) - Runtime.getRuntime().exec(command).waitFor() == 0 + ProcessBuilder(*command) + .redirectErrorStream(true) + .start() + .also { it.inputStream.bufferedReader().readText() } + .waitFor() == 0 } catch (e: Exception) { false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/build.gradle.kts` around lines 35 - 40, The isInstalled(binary: String) function currently calls Runtime.getRuntime().exec(command) without closing the returned Process's streams; update it to either use ProcessBuilder(command).inheritIO().start() or capture the Process, explicitly close its input, output and error streams (and call destroy()/waitFor()), so no stream handles are leaked; ensure you still check the exit code (process.waitFor() == 0) and handle exceptions as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@core/database/build.gradle.kts`:
- Around line 1-20: Remove the duplicated license header by keeping only a
single header block; locate the two consecutive license comment blocks (one
beginning "Copyright 2026 Mifos Initiative" and the other "Copyright 2025 Mifos
Initiative") at the top of core/database/build.gradle.kts and delete the
redundant one so only one correct license header remains.
---
Nitpick comments:
In `@core/database/build.gradle.kts`:
- Around line 35-40: The isInstalled(binary: String) function currently calls
Runtime.getRuntime().exec(command) without closing the returned Process's
streams; update it to either use ProcessBuilder(command).inheritIO().start() or
capture the Process, explicitly close its input, output and error streams (and
call destroy()/waitFor()), so no stream handles are leaked; ensure you still
check the exit code (process.waitFor() == 0) and handle exceptions as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2fe05f7a-ac58-4e22-8ba1-30cc2c006239
📒 Files selected for processing (1)
core/database/build.gradle.kts
niyajali
left a comment
There was a problem hiding this comment.
Implement all the required modifications, upload the videos for every platform, and once confirmed operational, transfer the configuration and setup to the core-base/sqldelight module. Structure it so that any new or existing project can utilize this database with the fewest possible configurations, specific to each project.
gradle/libs.versions.toml
Outdated
| sqliteBundled = "2.6.2" | ||
|
|
||
| sqldelight = "2.2.1" | ||
| coreKtx = "1.7.0" |
There was a problem hiding this comment.
This has already been defined on line 8, and should be renamed to androidxCore.
| import org.mifos.core.database.repository.SQLDelightSampleRepositoryImpl | ||
| import org.mifos.core.database.repository.SampleRepository | ||
|
|
||
| val DatabaseModule = module { |
There was a problem hiding this comment.
Please apply this module to the cmp-shared/KoinModules.kt file. Then, run the project on all supported platforms and provide a screenshot or video demonstrating the results across all of them.
core-base/database/build.gradle.kts
Outdated
| * | ||
| * See https://github.com/openMF/kmp-project-template/blob/main/LICENSE | ||
| */ | ||
| import org.gradle.declarative.dsl.schema.FqName.Empty.packageName |
There was a problem hiding this comment.
remove the changes from this file
… test execution * Standardize SQLDelight schema creation on Desktop using `.synchronous()`. * Fix race conditions in JS and WasmJS tests by awaiting schema initialization before test execution. * Update Webpack and Karma configurations to correctly resolve and serve `sql.js` WASM assets. * Integrate `DatabaseModule` into the shared navigation Koin module. * Refactor `libs.versions.toml` to rename `core` to `androidxCore` and clean up unused dependencies. * Update `core/database` build script with correct NPM dependencies for JS and WasmJS source sets.
…tion - Remove `MifosSQLDatabaseInitializer` and move schema creation logic directly into Koin modules. - Provide a `CoroutineScope` via Koin to handle asynchronous schema creation on JS and WASM platforms. - Update the Desktop driver to conditionally create the schema only if the database file is missing or empty. - Simplify JS and WASM unit tests by removing manual schema initialization tracking. - Add `sqldelight-coroutines` and `sqldelight-primitive-adapters` dependencies to `core:database`. - Update the Android driver to use `androidContext()` for initialization.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
core/database/src/desktopMain/kotlin/org/mifos/core/database/di/SQLDelightModule.desktop.kt (1)
22-29:⚠️ Potential issue | 🟠 MajorUse the JDBC driver's schema-aware path instead of the post-open file check.
JdbcSqliteDriveralready supports create/migrate fromuser_version, andSqlSchema.create()assumes an empty database. ThisFile.exists()/length()probe runs only after the connection has already been opened, so it is not a reliable signal for whether schema setup should run. PassMifosSQLDelightDatabase.Schema.synchronous()into the driver constructor and drop the manualSchema.create(driver)block. (sqldelight.github.io)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/src/desktopMain/kotlin/org/mifos/core/database/di/SQLDelightModule.desktop.kt` around lines 22 - 29, Replace the manual post-open file probe and explicit schema creation with the JDBC driver's schema-aware constructor: when constructing JdbcSqliteDriver, pass MifosSQLDelightDatabase.Schema.synchronous() (or the appropriate Schema instance via the driver's schema parameter) so the driver will handle create/migrate based on user_version; remove the File(DB_FILE_NAME).exists()/length() check and the MifosSQLDelightDatabase.Schema.create(driver) call entirely (locate these in the block that constructs JdbcSqliteDriver and the surrounding also { ... }).
🧹 Nitpick comments (5)
cmp-web/webpack.config.d/sqljs.js (2)
9-15: Consider usingrequire.resolvefor robust path resolution.The relative path
../../node_modules/sql.js/dist/sql-wasm.wasmis fragile and depends on webpack's context directory. The karma configs in this PR userequire.resolve("sql.js/dist/sql-wasm.wasm")for more reliable resolution across different execution contexts.♻️ Suggested fix
const CopyWebpackPlugin = require('copy-webpack-plugin'); config.plugins.push( new CopyWebpackPlugin({ patterns: [ - '../../node_modules/sql.js/dist/sql-wasm.wasm' + { from: require.resolve('sql.js/dist/sql-wasm.wasm') } ] }) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmp-web/webpack.config.d/sqljs.js` around lines 9 - 15, The CopyWebpackPlugin usage currently pushes a fragile relative path into config.plugins (new CopyWebpackPlugin with patterns: ['../../node_modules/sql.js/dist/sql-wasm.wasm']); change the pattern to use require.resolve('sql.js/dist/sql-wasm.wasm') so resolution is robust across contexts (update the patterns entry used in the new CopyWebpackPlugin instantiation and keep the plugin pushed to config.plugins as-is).
1-7: Merge with existingconfig.resolveinstead of overwriting.Assigning directly to
config.resolvediscards any prior resolve settings that may exist. Merge the fallback into the existing object to preserve other configuration.♻️ Suggested fix
-config.resolve = { - fallback: { +config.resolve = config.resolve || {}; +config.resolve.fallback = { + ...config.resolve.fallback, fs: false, path: false, crypto: false, - } -}; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmp-web/webpack.config.d/sqljs.js` around lines 1 - 7, The code overwrites config.resolve instead of merging fallbacks, which discards existing resolve settings; update the logic that sets config.resolve so it merges into any existing config.resolve (e.g., use Object.assign or spread) and merge or extend the fallback property rather than replacing it — ensure config.resolve = { ...(config.resolve || {}), fallback: { ...(config.resolve?.fallback || {}), fs: false, path: false, crypto: false } } so existing resolve settings are preserved while adding the fs/path/crypto fallbacks.cmp-navigation/build.gradle.kts (1)
24-27: Remove duplicatedprojects.core.datastoredependency entry.Line 24 and Line 27 declare the same dependency in the same source set. Keep a single declaration to avoid dependency block drift.
Suggested cleanup
commonMain.dependencies { // Core Modules implementation(projects.core.data) implementation(projects.core.model) implementation(projects.core.common) implementation(projects.core.datastore) implementation(projects.core.database) - implementation(projects.core.datastore) implementation(projects.coreBase.common) implementation(projects.coreBase.platform)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmp-navigation/build.gradle.kts` around lines 24 - 27, The dependencies block contains a duplicate declaration of implementation(projects.core.datastore); remove the redundant implementation(projects.core.datastore) entry so the dependency is declared only once (leave implementation(projects.core.database) and a single implementation(projects.core.datastore)), keeping the dependencies block clean to avoid drift.core/database/build.gradle.kts (2)
162-173: Remove the source-set dump before merge.
kotlin.sourceSets.all { println(...) }and the trailingprintln("jsMain depends on: ...")run during configuration on every Gradle invocation, so they permanently add noise to CI and local builds. If you still need this inspection, keep it behind thecheckSourceSetstask only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/build.gradle.kts` around lines 162 - 173, Remove the configuration-time printing that floods builds: delete the kotlin.sourceSets.all { println("SourceSet: $name") } block and the trailing println("jsMain depends on: ...") call so they no longer run during Gradle configuration; keep the existing tasks.register("checkSourceSets") task (and its doLast printing of source sets and dependsOn entries) as the only place that performs this inspection when explicitly executed.
105-156: Declaresqldelight-runtimeincommonMainand keep platform blocks for platform-only deps.
runtimeexposes common packages, but this file adds it separately to every platform source set while the shared SQLDelight wiring is already partly centralized inKMPSQLDelightConventionPlugin. That splits one shared dependency across five targets and makes the build logic easier to drift. Movelibs.sqldelight.runtimetocommonMainonce, and let the platform blocks focus on truly platform-specific additions. (sqldelight.github.io)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/database/build.gradle.kts` around lines 105 - 156, Move the sqldelight runtime dependency out of each platform source set and declare it once in commonMain: remove implementation(libs.sqldelight.runtime) from androidMain, nativeMain, desktopMain, jsMain, and wasmJsMain and add implementation(libs.sqldelight.runtime) to commonMain; keep platform-specific dependencies (e.g., androidx.room.runtime, androidx.sqlite.bundled, npm packages, etc.) in their respective source sets and leave KMPSQLDelightConventionPlugin wiring as the centralized/shared runtime configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/database/src/jsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.js.kt`:
- Around line 21-27: The singleton provider for SqlDriver currently returns the
driver immediately and launches
MifosSQLDelightDatabase.Schema.awaitCreate(driver) in a fire-and-forget
CoroutineScope, which can let consumers use the driver before tables exist;
change the initialization so awaitCreate(driver) completes before the driver is
exposed — e.g., convert the provider to an explicit suspend/bootstrap
initialization (or a blocking init at startup) that calls
createDefaultWebWorkerDriver(), then invokes and awaits
MifosSQLDelightDatabase.Schema.awaitCreate(driver) (using the same
CoroutineScope or a suspend call) and only after that registers/returns the
ready SqlDriver to consumers instead of returning it immediately.
In
`@core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.wasmJs.kt`:
- Around line 21-27: The singleton currently launches schema creation with
scope.launch and returns the driver immediately; change it to block until schema
creation completes by removing the fire-and-forget coroutine and calling a
blocking await before returning the driver (e.g., replace scope.launch {
MifosSQLDelightDatabase.Schema.awaitCreate(driver) } with runBlocking {
MifosSQLDelightDatabase.Schema.awaitCreate(driver) } or an equivalent
synchronous wait), so createDefaultWebWorkerDriver() is only returned after
MifosSQLDelightDatabase.Schema.awaitCreate(driver) finishes.
---
Duplicate comments:
In
`@core/database/src/desktopMain/kotlin/org/mifos/core/database/di/SQLDelightModule.desktop.kt`:
- Around line 22-29: Replace the manual post-open file probe and explicit schema
creation with the JDBC driver's schema-aware constructor: when constructing
JdbcSqliteDriver, pass MifosSQLDelightDatabase.Schema.synchronous() (or the
appropriate Schema instance via the driver's schema parameter) so the driver
will handle create/migrate based on user_version; remove the
File(DB_FILE_NAME).exists()/length() check and the
MifosSQLDelightDatabase.Schema.create(driver) call entirely (locate these in the
block that constructs JdbcSqliteDriver and the surrounding also { ... }).
---
Nitpick comments:
In `@cmp-navigation/build.gradle.kts`:
- Around line 24-27: The dependencies block contains a duplicate declaration of
implementation(projects.core.datastore); remove the redundant
implementation(projects.core.datastore) entry so the dependency is declared only
once (leave implementation(projects.core.database) and a single
implementation(projects.core.datastore)), keeping the dependencies block clean
to avoid drift.
In `@cmp-web/webpack.config.d/sqljs.js`:
- Around line 9-15: The CopyWebpackPlugin usage currently pushes a fragile
relative path into config.plugins (new CopyWebpackPlugin with patterns:
['../../node_modules/sql.js/dist/sql-wasm.wasm']); change the pattern to use
require.resolve('sql.js/dist/sql-wasm.wasm') so resolution is robust across
contexts (update the patterns entry used in the new CopyWebpackPlugin
instantiation and keep the plugin pushed to config.plugins as-is).
- Around line 1-7: The code overwrites config.resolve instead of merging
fallbacks, which discards existing resolve settings; update the logic that sets
config.resolve so it merges into any existing config.resolve (e.g., use
Object.assign or spread) and merge or extend the fallback property rather than
replacing it — ensure config.resolve = { ...(config.resolve || {}), fallback: {
...(config.resolve?.fallback || {}), fs: false, path: false, crypto: false } }
so existing resolve settings are preserved while adding the fs/path/crypto
fallbacks.
In `@core/database/build.gradle.kts`:
- Around line 162-173: Remove the configuration-time printing that floods
builds: delete the kotlin.sourceSets.all { println("SourceSet: $name") } block
and the trailing println("jsMain depends on: ...") call so they no longer run
during Gradle configuration; keep the existing tasks.register("checkSourceSets")
task (and its doLast printing of source sets and dependsOn entries) as the only
place that performs this inspection when explicitly executed.
- Around line 105-156: Move the sqldelight runtime dependency out of each
platform source set and declare it once in commonMain: remove
implementation(libs.sqldelight.runtime) from androidMain, nativeMain,
desktopMain, jsMain, and wasmJsMain and add
implementation(libs.sqldelight.runtime) to commonMain; keep platform-specific
dependencies (e.g., androidx.room.runtime, androidx.sqlite.bundled, npm
packages, etc.) in their respective source sets and leave
KMPSQLDelightConventionPlugin wiring as the centralized/shared runtime
configuration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47fcc23c-2a7f-4312-a6e5-faeaf500923b
📒 Files selected for processing (19)
cmp-navigation/build.gradle.ktscmp-navigation/src/commonMain/kotlin/cmp/navigation/di/KoinModules.ktcmp-web/karma.config.d/sqljs.jscmp-web/webpack.config.d/sqljs.jscore-base/database/build.gradle.ktscore/data/src/androidMain/kotlin/org/mifos/core/data/di/PlatformDependentDataModule.android.ktcore/database/build.gradle.ktscore/database/karma.config.d/sqljs-config.jscore/database/src/androidMain/kotlin/org/mifos/core/database/di/SQLDelightModule.android.ktcore/database/src/commonMain/kotlin/org/mifos/core/database/di/DatabaseModule.ktcore/database/src/commonMain/kotlin/org/mifos/core/database/di/SQLDelightModule.ktcore/database/src/commonTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.ktcore/database/src/desktopMain/kotlin/org/mifos/core/database/di/SQLDelightModule.desktop.ktcore/database/src/jsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.js.ktcore/database/src/jsTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.js.ktcore/database/src/nativeTest/kotlin/org/mifos/core/database/di/TestDatabaseModule.native.ktcore/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.wasmJs.ktcore/database/src/wasmJsTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.wasmJs.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (1)
- core/data/src/androidMain/kotlin/org/mifos/core/data/di/PlatformDependentDataModule.android.kt
🚧 Files skipped from review as they are similar to previous changes (6)
- core/database/src/jsTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.js.kt
- core-base/database/build.gradle.kts
- core/database/src/androidMain/kotlin/org/mifos/core/database/di/SQLDelightModule.android.kt
- core/database/src/commonMain/kotlin/org/mifos/core/database/di/DatabaseModule.kt
- core/database/src/wasmJsTest/kotlin/org/mifos/core/database/di/TestSQLDelightModule.wasmJs.kt
- cmp-web/karma.config.d/sqljs.js
| single<SqlDriver> { | ||
| val scope = get<CoroutineScope>() | ||
| val driver = createDefaultWebWorkerDriver() | ||
| scope.launch { | ||
| MifosSQLDelightDatabase.Schema.awaitCreate(driver) | ||
| } | ||
| driver |
There was a problem hiding this comment.
Wait for schema creation before exposing the web-worker driver.
This singleton returns the SqlDriver immediately and kicks awaitCreate off in a fire-and-forget launch, so the first consumer can build the database and query before the tables exist. SQLDelight's multiplatform web-worker setup awaits schema creation before returning the driver; make initialization an explicit suspend/bootstrap step and only register a ready driver/database after it completes. (sqldelight.github.io)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/database/src/jsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.js.kt`
around lines 21 - 27, The singleton provider for SqlDriver currently returns the
driver immediately and launches
MifosSQLDelightDatabase.Schema.awaitCreate(driver) in a fire-and-forget
CoroutineScope, which can let consumers use the driver before tables exist;
change the initialization so awaitCreate(driver) completes before the driver is
exposed — e.g., convert the provider to an explicit suspend/bootstrap
initialization (or a blocking init at startup) that calls
createDefaultWebWorkerDriver(), then invokes and awaits
MifosSQLDelightDatabase.Schema.awaitCreate(driver) (using the same
CoroutineScope or a suspend call) and only after that registers/returns the
ready SqlDriver to consumers instead of returning it immediately.
| single<SqlDriver> { | ||
| val scope = get<CoroutineScope>() | ||
| val driver = createDefaultWebWorkerDriver() | ||
| scope.launch { | ||
| MifosSQLDelightDatabase.Schema.awaitCreate(driver) | ||
| } | ||
| driver |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and examine its full content
find . -path "*/core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.wasmJs.kt" -type fRepository: openMF/kmp-project-template
Length of output: 162
🏁 Script executed:
# Also check for the JS version to compare patterns
find . -path "*/core/database/src/jsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.js.kt" -type fRepository: openMF/kmp-project-template
Length of output: 154
🏁 Script executed:
# Read the wasmJs module
cat -n ./core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.wasmJs.ktRepository: openMF/kmp-project-template
Length of output: 1260
🏁 Script executed:
# Read the JS module for comparison
cat -n ./core/database/src/jsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.js.ktRepository: openMF/kmp-project-template
Length of output: 1260
🏁 Script executed:
# Check for other platform-specific driver modules
find . -path "*/core/database/src/*/kotlin/org/mifos/core/database/di/SQLDelightModule*" -type fRepository: openMF/kmp-project-template
Length of output: 611
🏁 Script executed:
# Search for how the driver singleton is used/injected in the codebase
rg "get<SqlDriver>" --type kt -A 3 -B 3Repository: openMF/kmp-project-template
Length of output: 97
🏁 Script executed:
# Search for MifosSQLDelightDatabase instantiation/usage
rg "MifosSQLDelightDatabase" --type kt -A 2 -B 2 | head -100Repository: openMF/kmp-project-template
Length of output: 97
🏁 Script executed:
# Search for how the driver singleton is used, without -t flag
rg "get<SqlDriver>" -A 3 -B 3Repository: openMF/kmp-project-template
Length of output: 53
🏁 Script executed:
# Check the commonMain module
cat -n ./core/database/src/commonMain/kotlin/org/mifos/core/database/di/SQLDelightModule.ktRepository: openMF/kmp-project-template
Length of output: 1192
🏁 Script executed:
# Search for database instantiation patterns
rg "MifosSQLDelightDatabase\(" -A 5 -B 2Repository: openMF/kmp-project-template
Length of output: 1703
🏁 Script executed:
# Check other platform implementations to see how they handle schema creation
cat -n ./core/database/src/androidMain/kotlin/org/mifos/core/database/di/SQLDelightModule.android.ktRepository: openMF/kmp-project-template
Length of output: 1181
🏁 Script executed:
# Check the native implementation
cat -n ./core/database/src/nativeMain/kotlin/org/mifos/core/database/di/SQLDelightModule.native.ktRepository: openMF/kmp-project-template
Length of output: 1086
🏁 Script executed:
# Check the desktop implementation
cat -n ./core/database/src/desktopMain/kotlin/org/mifos/core/database/di/SQLDelightModule.desktop.ktRepository: openMF/kmp-project-template
Length of output: 1336
🏁 Script executed:
# Check how the database is used/accessed in the codebase
rg "single<MifosSQLDelightDatabase>" -A 10 -B 5Repository: openMF/kmp-project-template
Length of output: 2516
🏁 Script executed:
# Search for any initialization or bootstrap code that might wait for schema creation
rg "awaitCreate" --context 5Repository: openMF/kmp-project-template
Length of output: 12812
🏁 Script executed:
# Check if there are any tests or usage patterns that show when the database is first accessed
rg "get<MifosSQLDelightDatabase>" -A 5 -B 2Repository: openMF/kmp-project-template
Length of output: 53
The schema creation must complete before exposing the driver to consumers.
The singleton launches awaitCreate in a fire-and-forget coroutine and immediately returns the driver. Any code that injects this driver can query before the schema creation completes, causing failures. SQLDelight's multiplatform drivers (Android, Native) use synchronous schema creation; the desktop test module blocks with runBlocking { awaitCreate(...) } until ready. Change this to explicitly await schema completion before the driver is returned, similar to the desktop test pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/database/src/wasmJsMain/kotlin/org/mifos/core/database/di/SQLDelightModule.wasmJs.kt`
around lines 21 - 27, The singleton currently launches schema creation with
scope.launch and returns the driver immediately; change it to block until schema
creation completes by removing the fire-and-forget coroutine and calling a
blocking await before returning the driver (e.g., replace scope.launch {
MifosSQLDelightDatabase.Schema.awaitCreate(driver) } with runBlocking {
MifosSQLDelightDatabase.Schema.awaitCreate(driver) } or an equivalent
synchronous wait), so createDefaultWebWorkerDriver() is only returned after
MifosSQLDelightDatabase.Schema.awaitCreate(driver) finishes.
Jira Ticket: KMPPR-91
Screenshots
Android
JVM
js
wasmJs
Summary
This PR integrates SQLDelight as the primary database solution for the Mifos KMP project template. It establishes a robust, type-safe, and offline-first data layer compatible across Android, Desktop (JVM), iOS/Native, and Web (JS/Wasm) platforms. This implementation provides the foundation for the generic offline-first data
layer module using Store5.
Key Changes
🏗️ Core Infrastructure
📱 Multiplatform Driver Support
🧪 Testing & Validation
🧹 Code Quality & Maintenance
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores