Replace manual dependency injections with Koin

[283375]

Also refactored some of the design flaws

[283375]

/opencode /review pr

[opencode-agent]

PR Review: Replace manual dependency injections with Koin

59 files, +882/-860, 9 commits

Summary

Solid migration from manual DI (container classes + AppViewModelProvider) to Koin 4.2.1. The refactor is thorough — all view models, workers, repos, daos, and databases are now managed by Koin modules. A bonus ImportLog extraction improves separation of concerns.

---

Positive Findings

Clean DI module structure — CoreModule provides the core database layer; AppModule includes it and adds app-specific bindings. The module separation matches the project's core/app structure well.

Structured log events (ImportLog.kt:14-28) — The ImportLogEvent sealed interface (Plural/SimpleString/Raw) replaces raw Resources.getString() calls in the ViewModel. This is a nice design improvement — the ViewModel no longer depends on Resources/AssetManager.

SupervisorJob fix (ArcaeaOfflineApplication.kt:52) — Adding SupervisorJob() to the application scope prevents one failed child from cancelling siblings. Good catch.

Database schema version via MetaDao — The new MetaDao with @RawQuery("PRAGMA user_version") replaces the ad-hoc usePrepared("PRAGMA user_version;") in the old factory. Cleaner abstraction.

---

Concerns

Medium: getDatabase() no longer caches instances

In all 3 databases (AppDatabase, OcrQueueDatabase, ArcaeaOfflineDatabase), the @Volatile private var instance + synchronized double-checked locking was removed. Now getDatabase() creates a new instance on every call. This relies entirely on Koin's single scope:

// Before (ArcaeaOfflineDatabase.kt):
return instance ?: synchronized(this) { ... .also { instance = it } }

// After:
return getDatabaseBuilder(context) ... .build()  // always builds

This is fine in production since Koin's single calls getDatabase() once. However, any code outside Koin scope that calls getDatabase() will now create duplicate instances. Consider keeping the singleton guard internally and using Koin's single for belt-and-suspenders safety.

Medium: getDatabaseBuilder was made public

Previously private fun getDatabaseBuilder(), now fun (implicitly public). This was necessary for Koin's factory functions. It leaks an internal implementation detail — consider making it internal at minimum.

Low: ArcaeaPackageHelper now constructed inside sendTask

In importArcaeaApkFromInstalled, the helper was previously a lazy property; now it's constructed each time the task runs. This is fine in practice but is a behavioral change.

---

Verdict

LGTM. The DI migration is clean and consistent across all layers. The ImportLog extraction is thoughtful. No blocking issues — the database singleton observation is worth a follow-up but not a blocker.

opencode session  |  github run