-
Notifications
You must be signed in to change notification settings - Fork 0
UseCase/Repository/Testの見直し #301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough新しい BackupRepository インターフェースと BackupRepositoryImpl を追加し、DI にバインド。PackageManager と Context をコンストラクタ注入へ移行し、複数の UseCase と ViewModel の公開メソッド署名を引数削除で簡素化。リポジトリとユースケースのテストを追加/更新。 Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI / ViewModel
participant UC as Export/Import UseCase
participant BR as BackupRepositoryImpl
participant CTX as Android Context (contentResolver)
UI->>UC: invoke(uri)
UC->>UC: serialize/deserialize JSON
UC->>BR: exportToUri(uri, json) / importFromUri(uri)
BR->>CTX: contentResolver.openOutputStream / openInputStream(uri)
CTX-->>BR: Stream
BR->>BR: write/read (IO on dispatcher)
BR-->>UC: completion / json
UC-->>UI: Result<Unit>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (2)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/SettingsViewModel.kt (4)
AndroidApp/data/repository/build.gradle.kts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/AppRepositoryImpl.kt (1)
66-76: AppRepository インターフェースとその実装を suspend に統一する必要があります
loadInstalledAppList()はpackageManager.getInstalledApplications()とpackageManager.getApplicationLabel()という I/O を伴うブロッキング呼び出しを実行していますが、suspend関数ではありません。同じクラスの他のすべてのメソッドはsuspendでwithContext(coroutineDispatcher)を使用しており、一貫性がありません。現在の実装は
LoadAppUseCaseImpl経由で呼び出される場合、invoke()のwithContext()内で実行されるため実際には保護されていますが、このパターンに依存するのは脆弱です。リポジトリのインターフェースが I/O ブロッキング呼び出しを直接公開している設計は改善が必要です。以下の変更が必要です:
- AppRepository インターフェース (line 25) を
suspendに変更- AppRepositoryImpl の実装を
suspend+withContextで保護🔧 提案される修正
--- a/AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/AppRepository.kt +++ b/AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/AppRepository.kt @@ -22,7 +22,7 @@ interface AppRepository { suspend fun addTargetApp(target: InstalledApp) suspend fun deleteTargetApp(target: InstalledApp) - fun loadInstalledAppList(): List<InstalledApp> + suspend fun loadInstalledAppList(): List<InstalledApp>--- a/AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/AppRepositoryImpl.kt +++ b/AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/AppRepositoryImpl.kt @@ -63,7 +63,8 @@ class AppRepositoryImpl( } } - override fun loadInstalledAppList(): List<InstalledApp> { - return packageManager.getInstalledApplications(PackageManager.GET_META_DATA) + override suspend fun loadInstalledAppList(): List<InstalledApp> { + return withContext(coroutineDispatcher) { + packageManager.getInstalledApplications(PackageManager.GET_META_DATA) .map { val label = packageManager.getApplicationLabel(it).toString() InstalledApp( @@ -73,6 +74,7 @@ class AppRepositoryImpl( ) } .sortedBy { it.label } + } }また、
LoadAppUseCaseImpl.loadInstalledAppList()(line 30-37) も suspend に変更して、awaitなしで呼び出しできるようにしてください。
🤖 Fix all issues with AI agents
In `@AndroidApp/data/repository/build.gradle.kts`:
- Around line 23-29: Remove the unnecessary Espresso dependency from
instrumented test dependencies: delete the
androidTestImplementation(libs.androidx.test.espresso.core) entry in the
dependencies block (it is not used by AppRepositoryTest.kt or
UserSettingsRepositoryTest.kt and is only needed for UI tests), leaving junit,
truth, androidx.test.ext.junit, and kotlinx.coroutines.test in place to reduce
build time and memory.
In
`@AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/BackupRepositoryImpl.kt`:
- Around line 24-34: importFromUri incorrectly relies on a post-`use` Elvis and
appends a possible null line; change it to (inside
BackupRepositoryImpl.importFromUri) open the input stream with
context.contentResolver.openInputStream(uri) and throw if null, then use
BufferedReader(InputStreamReader(input)).use { reader -> val text =
reader.readText(); if (text.isEmpty()) throw RuntimeException("Empty input
stream") else return text } so you don't depend on reader.readLine() returning
non-null and you remove the ineffective `?:` after `use`.
In
`@AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/ExportDataUseCase.kt`:
- Line 7: SettingsViewModel still calls exportDataUseCase with two args but
ExportDataUseCase's operator fun invoke signature changed to suspend operator
fun invoke(uri: Uri): Result<Unit>; update the call in SettingsViewModel (the
site that currently calls exportDataUseCase(context, uri)) to call
exportDataUseCase(uri) instead, removing the context parameter and adjusting any
surrounding coroutine/scope usage to match the single-argument suspend function
(follow the pattern used in UseCaseTest where exportDataUseCase(uri) is
invoked).
In
`@AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/ImportDataUseCaseImpl.kt`:
- Line 17: SettingsViewModel still calls importDataUseCase with two args
(context, uri) but ImportDataUseCaseImpl now defines operator fun invoke(uri:
Uri): Result<Unit>; update the call in SettingsViewModel (the
importDataUseCase(...) invocation) to pass only the Uri
(importDataUseCase(uri)), or alternatively adjust the use case interface back to
accept Context if intended—ensure function names match
ImportDataUseCaseImpl.invoke and remove the extra context argument.
In
`@AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/ImportDataUseCase.kt`:
- Around line 6-7: The SettingsViewModel still calls the old signature
importDataUseCase(context, uri) but ImportDataUseCase now exposes suspend
operator fun invoke(uri: Uri): Result<Unit>; update the call in
SettingsViewModel (the place invoking importDataUseCase) to pass only the Uri
(importDataUseCase(uri)) and remove the Context parameter, and ensure any
surrounding coroutine/scope usage around the invoke call remains correct.
In `@AndroidApp/gradle/libs.versions.toml`:
- Line 35: The mockk dependency entry currently pins mockk to "1.14.7"; update
the mockk version string in libs.versions.toml (the mockk entry) to the stable
release "1.14.6" so the project uses the recommended MockK core version for
JVM/unit tests while keeping mockk-android unchanged.
🧹 Nitpick comments (4)
AndroidApp/data/repository/src/androidTest/kotlin/me/nya_n/notificationnotifier/UserSettingsRepositoryTest.kt (1)
22-23:testDispatcherは省略可能です。
runTestはデフォルトでStandardTestDispatcherを使用しますが、UnconfinedTestDispatcherを明示的に渡すことで即時実行の挙動を得られます。現在の実装で問題はありませんが、runTestのデフォルト挙動で十分であれば、この宣言を省略してコードを簡潔にできます。AndroidApp/domain/src/test/java/me/nya_n/notificationnotifier/Annotations.kt (1)
1-5: 重複するアノテーション定義
relevant_code_snippetsによると、同じLocalOnlyアノテーションがandroidTestソースセット(AndroidApp/domain/src/androidTest/java/me/nya_n/notificationnotifier/Annotations.kt)にも存在しています。
testとandroidTestの両方で使用する場合は、共通モジュールに配置するか、testFixturesを活用して重複を排除することを検討してください。#!/bin/bash # LocalOnlyアノテーションの重複を確認 fd -e kt Annotations.kt --exec cat {}AndroidApp/domain/build.gradle.kts (1)
33-34: androidTestでのmockk依存関係の重複可能性
mockk-androidはmockkを推移的依存関係として含んでいます。Line 33のlibs.mockkは冗長かもしれません。♻️ 提案される修正
- androidTestImplementation(libs.mockk) androidTestImplementation(libs.mockk.android)AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/ImportDataUseCaseImpl.kt (1)
21-22: エラーメッセージの改善を検討してください。バージョン不一致時のエラーメッセージがデバッグに役立ちません。期待値と実際の値を含めると問題の特定が容易になります。
💡 提案
if (backup.version != DB.version()) { - throw RuntimeException("bad version.") + throw RuntimeException("Backup version mismatch: expected ${DB.version()}, got ${backup.version}") }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
AndroidApp/app/src/main/kotlin/me/nya_n/notificationnotifier/App.ktAndroidApp/data/repository/build.gradle.ktsAndroidApp/data/repository/src/androidTest/kotlin/me/nya_n/notificationnotifier/AppRepositoryTest.ktAndroidApp/data/repository/src/androidTest/kotlin/me/nya_n/notificationnotifier/UserSettingsRepositoryTest.ktAndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/AppRepository.ktAndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/BackupRepository.ktAndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/AppRepositoryImpl.ktAndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/BackupRepositoryImpl.ktAndroidApp/domain/build.gradle.ktsAndroidApp/domain/src/androidTest/java/me/nya_n/notificationnotifier/UseCaseTest.ktAndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/ExportDataUseCase.ktAndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/ImportDataUseCase.ktAndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/LoadAppUseCase.ktAndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/dummy/LoadAppUseCaseDummyImpl.ktAndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/ExportDataUseCaseImpl.ktAndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/ImportDataUseCaseImpl.ktAndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/LoadAppUseCaseImpl.ktAndroidApp/domain/src/test/java/me/nya_n/notificationnotifier/Annotations.ktAndroidApp/domain/src/test/java/me/nya_n/notificationnotifier/UseCaseTest.ktAndroidApp/gradle/libs.versions.tomlAndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/selection/SelectionViewModel.ktAndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/target/TargetViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (14)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/ImportDataUseCase.kt (1)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/SettingsViewModel.kt (1)
importData(80-93)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/LoadAppUseCase.kt (1)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/PackageVisibilityGrantedUseCase.kt (2)
invoke(3-5)invoke(4-4)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/target/TargetViewModel.kt (1)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/target/TargetScreen.kt (2)
viewModel(30-32)TargetScreen(23-45)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/LoadAppUseCaseImpl.kt (2)
AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/AppRepository.kt (1)
loadInstalledAppList(25-25)AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/AppRepositoryImpl.kt (1)
loadInstalledAppList(66-76)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/dummy/LoadAppUseCaseDummyImpl.kt (1)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/dummy/LoadAddressUseCaseDummyImpl.kt (2)
invoke(6-8)invoke(5-9)
AndroidApp/gradle/libs.versions.toml (2)
AndroidApp/build-logic/build.gradle.kts (1)
compileOnly(5-7)AndroidApp/build-logic/settings.gradle.kts (1)
repositories(3-14)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/ExportDataUseCase.kt (1)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/SettingsViewModel.kt (1)
exportData(64-77)
AndroidApp/domain/src/test/java/me/nya_n/notificationnotifier/Annotations.kt (1)
AndroidApp/domain/src/androidTest/java/me/nya_n/notificationnotifier/Annotations.kt (1)
AnnotationTarget(3-5)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/ImportDataUseCaseImpl.kt (1)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/SettingsViewModel.kt (1)
importData(80-93)
AndroidApp/data/repository/src/androidTest/kotlin/me/nya_n/notificationnotifier/AppRepositoryTest.kt (4)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/NotifyTargetAppNotificationUseCaseImpl.kt (1)
appRepository(8-40)AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/AddTargetAppUseCaseImpl.kt (1)
appRepository(7-13)AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/DeleteTargetAppUseCaseImpl.kt (1)
appRepository(7-13)AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/SaveFilterConditionUseCaseImpl.kt (1)
appRepository(8-22)
AndroidApp/domain/build.gradle.kts (1)
AndroidApp/ui/build.gradle.kts (1)
implementation(17-35)
AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/BackupRepository.kt (3)
AndroidApp/model/src/main/kotlin/me/nya_n/notificationnotifier/model/Backup.kt (1)
setting(3-12)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/UiEvent.kt (1)
class ImportData : UiEvent()(10-10)AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/SettingsViewModel.kt (1)
importData(80-93)
AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/BackupRepositoryImpl.kt (1)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/SettingsViewModel.kt (1)
importData(80-93)
AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/AppRepositoryImpl.kt (2)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/LoadFilterConditionUseCaseImpl.kt (1)
appRepository(8-14)AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/AddTargetAppUseCaseImpl.kt (1)
appRepository(7-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (25)
AndroidApp/data/repository/src/androidTest/kotlin/me/nya_n/notificationnotifier/UserSettingsRepositoryTest.kt (2)
26-41: LGTM!テストの分離が適切に行われています。
@Beforeで SharedPreferences をクリアすることで、各テスト実行前にクリーンな状態を確保しています。
43-54: 基本的なテストカバレッジは良好です。保存、取得、更新の基本フローが適切にテストされています。将来的には以下のエッジケースのテスト追加を検討してください:
- 初期状態(保存前)の
getUserSettings()の挙動- 境界値(空文字の IP アドレス、ポート番号の範囲外の値など)
AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/AppRepository.kt (1)
25-25: LGTM! インターフェースの簡素化
PackageManagerパラメータを削除し、実装側でDI経由で注入する設計は適切です。これによりインターフェースがよりクリーンになり、テスト容易性も向上します。AndroidApp/domain/build.gradle.kts (1)
24-27: テスト依存関係の追加 - LGTMユニットテスト用の依存関係は適切です:
- JUnit: テストフレームワーク
- Truth: アサーションライブラリ
- kotlinx-coroutines-test: コルーチンテスト用
- MockK: モッキング用
AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/AppRepositoryImpl.kt (1)
14-14: コンストラクタインジェクション - LGTM
PackageManagerをコンストラクタで注入することで、メソッドシグネチャがシンプルになり、テスト時にモック可能になります。AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/BackupRepository.kt (1)
5-17: LGTM!バックアップ操作のための明確で簡潔なインターフェース設計です。
suspend関数の使用はI/O操作に適切で、プラットフォーム依存性をリポジトリ層に適切に隠蔽しています。AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/LoadAppUseCaseImpl.kt (2)
20-27: LGTM!
PackageManagerをユースケースのシグネチャから削除し、リポジトリ層に委譲する変更は適切です。withContextによるディスパッチャ制御とResult型によるエラーハンドリングが維持されています。
29-37: パーミッションチェックのロジックが適切に維持されています。
@VisibleForTestingアノテーションを維持しながら、パラメータを削除した変更は正しいです。userSettingsRepository経由のパーミッション確認ロジックがそのまま保持されています。AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/LoadAppUseCase.kt (1)
5-12: クリーンアーキテクチャに沿った良い改善です。ドメイン層のインターフェースから
PackageManager(Androidフレームワーク依存)を除去し、プラットフォーム非依存にしています。これによりテスタビリティが向上し、関心の分離が明確になります。AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/selection/SelectionViewModel.kt (1)
16-38: LGTM!ViewModelから
ContextとPackageManagerの依存を除去し、ユースケースのみに依存する形に改善されています。これによりViewModelのテストが容易になり、責務が明確になります。AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/dummy/LoadAppUseCaseDummyImpl.kt (1)
6-27: LGTM!ダミー実装が更新されたインターフェースに正しく準拠しています。UIプレビュー用のサンプルデータとして適切です。
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/target/TargetViewModel.kt (2)
13-15: LGTM! コンストラクタの簡素化ContextをViewModelのコンストラクタから削除し、UseCase経由で依存性を注入するリファクタリングは適切です。これによりViewModelのテスタビリティが向上します。
20-31: エラーハンドリングの確認
useCase()の呼び出しでonSuccessのみ処理していますが、onFailureの場合はエラーメッセージが表示されません。現状の実装では失敗時にローディングが終了するだけです。これが意図した動作であれば問題ありませんが、ユーザーにエラーを通知する必要がある場合は、
onFailureハンドラの追加を検討してください。AndroidApp/data/repository/src/androidTest/kotlin/me/nya_n/notificationnotifier/AppRepositoryTest.kt (3)
25-37: LGTM! テストセットアップインメモリデータベースとテスト用ディスパッチャを使用したセットアップは適切です。
clearAllTables()でテスト間の独立性を確保しています。
39-53: LGTM! 通知対象アプリのCRUDテスト追加、取得、削除のフローが適切にテストされています。
56-81: LGTM! 通知条件のCRUDテスト初期状態、追加、更新のフローが網羅的にテストされています。
getFilterConditionとgetFilterConditionOrDefaultの両方の動作が検証されています。AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/BackupRepositoryImpl.kt (1)
16-22: LGTM!exportToUriの実装出力ストリームの処理とエラーハンドリングは適切です。
AndroidApp/domain/src/test/java/me/nya_n/notificationnotifier/UseCaseTest.kt (3)
37-48: LGTM! テストセットアップUseCase のモック設定と
TextUtils.isDigitsOnlyのスタティックモックは適切です。JVMテストでAndroidフレームワークの依存を回避する良いパターンです。
50-72: LGTM! インストール済みアプリ取得のテスト許可あり/なしの両方のシナリオが適切にテストされています。
108-124: LGTM! ローカル専用テストのパターン
@LocalOnlyアノテーションとassumeTrueを使用してCI環境でスキップする実装は適切です。実際のネットワーク通信を伴うテストの管理方法として良いパターンです。AndroidApp/domain/src/androidTest/java/me/nya_n/notificationnotifier/UseCaseTest.kt (2)
62-104: LGTM!テストセットアップが新しい
BackupRepositoryパターンに正しく更新されています。BackupRepositoryImplの作成と、エクスポート/インポートのユースケースへの依存性注入が適切に行われています。
106-150: テストケースが新しいシグネチャに正しく対応しています。バックアップ・復元のテストが
Uriのみを引数として渡すように更新され、ターゲット、フィルター条件、アドレスの復元を適切に検証しています。AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/ExportDataUseCaseImpl.kt (1)
12-28: LGTM!I/O処理を
BackupRepositoryに委譲することで、責務の分離が改善されています。CoroutineDispatcherの管理をリポジトリ層に移動したことで、ユースケースがシンプルになりました。AndroidApp/app/src/main/kotlin/me/nya_n/notificationnotifier/App.kt (2)
86-87: LGTM!新しい
BackupRepositoryのDIバインディングが適切に追加されています。AppRepositoryへのPackageManager注入と、バックアップ機能のリポジトリ分離により、依存関係が明確になりました。
99-100: LGTM!エクスポート/インポートのユースケースが新しい3つの依存関係(
UserSettingsRepository,AppRepository,BackupRepository)で正しくバインドされています。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| override suspend fun importFromUri(uri: Uri): String { | ||
| val sb = StringBuilder() | ||
| withContext(coroutineDispatcher) { | ||
| context.contentResolver.openInputStream(uri)?.use { input -> | ||
| BufferedReader(InputStreamReader(input)).use { reader -> | ||
| sb.append(reader.readLine()) | ||
| } ?: throw RuntimeException("Failed to read input stream.") | ||
| } ?: throw RuntimeException("Failed to open input stream.") | ||
| } | ||
| return sb.toString() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
importFromUriの実装に問題があります
2つの問題があります:
-
Line 30のnullチェックが機能しない:
BufferedReader.use { }は常にラムダの結果(sb.append()の戻り値であるStringBuilder)を返すため、?: throw RuntimeException("Failed to read input stream.")は決して実行されません。 -
readLine()がnullを返す場合の処理: ストリームが空の場合、reader.readLine()はnullを返し、sb.append(null)は文字列"null"を追加してしまいます。
🐛 修正案
override suspend fun importFromUri(uri: Uri): String {
- val sb = StringBuilder()
withContext(coroutineDispatcher) {
context.contentResolver.openInputStream(uri)?.use { input ->
BufferedReader(InputStreamReader(input)).use { reader ->
- sb.append(reader.readLine())
- } ?: throw RuntimeException("Failed to read input stream.")
+ return@withContext reader.readLine()
+ ?: throw RuntimeException("Failed to read input stream.")
+ }
} ?: throw RuntimeException("Failed to open input stream.")
}
- return sb.toString()
}🤖 Prompt for AI Agents
In
`@AndroidApp/data/repository/src/main/kotlin/me/nya_n/notificationnotifier/data/repository/impl/BackupRepositoryImpl.kt`
around lines 24 - 34, importFromUri incorrectly relies on a post-`use` Elvis and
appends a possible null line; change it to (inside
BackupRepositoryImpl.importFromUri) open the input stream with
context.contentResolver.openInputStream(uri) and throw if null, then use
BufferedReader(InputStreamReader(input)).use { reader -> val text =
reader.readText(); if (text.isEmpty()) throw RuntimeException("Empty input
stream") else return text } so you don't depend on reader.readLine() returning
non-null and you remove the ineffective `?:` after `use`.
| /** バックアップのために外部ストレージにデータを保存 */ | ||
| interface ExportDataUseCase { | ||
| suspend operator fun invoke(context: Context, uri: Uri): Result<Unit> | ||
| suspend operator fun invoke(uri: Uri): Result<Unit> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find and examine ExportDataUseCase interface
find . -name "ExportDataUseCase.kt" -type f | head -20Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 180
🏁 Script executed:
# Search for all calls to exportDataUseCase
rg -n 'exportDataUseCase\(' --type=kt -C3Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 105
🏁 Script executed:
# Also search for invocations using invoke operator
rg -n 'invoke.*[Uu]ri' --type=kt | grep -i exportRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 105
🏁 Script executed:
# Read the ExportDataUseCase.kt file
cat -n ./AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/ExportDataUseCase.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 332
🏁 Script executed:
# Search for all calls to exportDataUseCase without file type restriction
rg 'exportDataUseCase' -n -C3Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 4895
🏁 Script executed:
# Find SettingsViewModel
find . -name "SettingsViewModel.kt" -type fRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 180
SettingsViewModelの呼び出し箇所を新しいシグネチャに更新してください。
インターフェースのシグネチャが invoke(uri: Uri) に変更されましたが、SettingsViewModel.kt (line 70) では依然として exportDataUseCase(context, uri) と2つの引数で呼び出しています。これはコンパイルエラーになります。UseCaseTest.kt (line 124) は正しく exportDataUseCase(uri) と1つの引数で呼び出しているため、SettingsViewModelも同様に更新が必要です。
🤖 Prompt for AI Agents
In
`@AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/ExportDataUseCase.kt`
at line 7, SettingsViewModel still calls exportDataUseCase with two args but
ExportDataUseCase's operator fun invoke signature changed to suspend operator
fun invoke(uri: Uri): Result<Unit>; update the call in SettingsViewModel (the
site that currently calls exportDataUseCase(context, uri)) to call
exportDataUseCase(uri) instead, removing the context parameter and adjusting any
surrounding coroutine/scope usage to match the single-argument suspend function
(follow the pattern used in UseCaseTest where exportDataUseCase(uri) is
invoked).
| private val backupRepository: BackupRepository, | ||
| ) : ImportDataUseCase { | ||
| override suspend operator fun invoke(context: Context, uri: Uri): Result<Unit> { | ||
| override suspend operator fun invoke(uri: Uri): Result<Unit> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SettingsViewModelの呼び出しが更新されていません。
SettingsViewModel.kt (line 85) では依然として importDataUseCase(context, uri) と2つの引数で呼び出しています。インターフェースの変更に合わせて更新が必要です。
🤖 Prompt for AI Agents
In
`@AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/ImportDataUseCaseImpl.kt`
at line 17, SettingsViewModel still calls importDataUseCase with two args
(context, uri) but ImportDataUseCaseImpl now defines operator fun invoke(uri:
Uri): Result<Unit>; update the call in SettingsViewModel (the
importDataUseCase(...) invocation) to pass only the Uri
(importDataUseCase(uri)), or alternatively adjust the use case interface back to
accept Context if intended—ensure function names match
ImportDataUseCaseImpl.invoke and remove the extra context argument.
| interface ImportDataUseCase { | ||
| suspend operator fun invoke(context: Context, uri: Uri): Result<Unit> | ||
| suspend operator fun invoke(uri: Uri): Result<Unit> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for importDataUseCase usages to verify call sites are updated
rg -n "importDataUseCase" --type=kotlin -C 3Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 4694
インターフェースの変更は適切ですが、呼び出し元の更新が必要です
Contextパラメータの削除によりUseCaseがAndroidフレームワークから独立し、テスタビリティが向上します。
ただし、SettingsViewModel.kt (Line 86) で importDataUseCase(context, uri) と呼び出されている箇所が、新しいシグネチャ importDataUseCase(uri) に更新されていません。このままではコンパイルエラーが発生します。SettingsViewModelの呼び出しを更新してください。
参考: UseCaseTest.kt (Line 134) は正しく新しいシグネチャで呼び出されています。
🤖 Prompt for AI Agents
In
`@AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/ImportDataUseCase.kt`
around lines 6 - 7, The SettingsViewModel still calls the old signature
importDataUseCase(context, uri) but ImportDataUseCase now exposes suspend
operator fun invoke(uri: Uri): Result<Unit>; update the call in
SettingsViewModel (the place invoking importDataUseCase) to pass only the Uri
(importDataUseCase(uri)) and remove the Context parameter, and ensure any
surrounding coroutine/scope usage around the invoke call remains correct.
| io-insert-koin = "4.1.1" | ||
| com-google-code-gson = "2.13.2" | ||
| com-squareup-leakcanary = "2.14" | ||
| mockk = "1.14.7" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
MockK latest version 2025 2026 Kotlin
💡 Result:
As of Jan 14, 2026:
- MockK (core/jvm/dsl) latest stable version: 1.14.6 (released Oct 2025). [1][3]
- Some MockK artifacts (e.g., mockk-bdd-android) have 1.14.7 (Dec 6, 2025). [2]
Sources: [1] javadoc.io listing (mockk 1.14.6). [2] Maven Central entry for mockk-bdd-android 1.14.7. [3] Maven repo listing for mockk 1.14.6.
MockK依存関係の追加を確認
MockKの設定アプローチは適切です。mockk(ユニットテスト用)とmockk-android(インストルメンテッドテスト用)の両方を定義しているのは正しい方法です。ただし、MockKコア(mockk jvm/dsl)の最新安定版は1.14.6(2025年10月リリース)です。バージョンを1.14.6に更新することを推奨します。
🤖 Prompt for AI Agents
In `@AndroidApp/gradle/libs.versions.toml` at line 35, The mockk dependency entry
currently pins mockk to "1.14.7"; update the mockk version string in
libs.versions.toml (the mockk entry) to the stable release "1.14.6" so the
project uses the recommended MockK core version for JVM/unit tests while keeping
mockk-android unchanged.
Summary by CodeRabbit
新機能
変更
テスト
✏️ Tip: You can customize this high-level summary in your review settings.