Skip to content
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

🔧 SQLDelight の導入 #146

Merged
merged 3 commits into from
Dec 11, 2023
Merged

🔧 SQLDelight の導入 #146

merged 3 commits into from
Dec 11, 2023

Conversation

tatsutakein
Copy link
Member

@tatsutakein tatsutakein commented Dec 11, 2023

Issue

  • close #ISSUE_NUMBER 🦕

概要

SQLDelight を導入します。

レビュー観点

特になし

レビューレベル

  • Lv0: まったく見ないで Approve する
  • Lv1: ぱっとみて違和感がないかチェックして Approve する
  • Lv2: 仕様レベルまで理解して、仕様通りに動くかある程度検証して Approve する
  • Lv3: 実際に環境で動作確認したうえで Approve する

レビュー優先度

  • すぐに見てもらいたい ( hotfix など ) 🚀
  • 今日中に見てもらいたい 🚗
  • 今日〜明日中で見てもらいたい 🚶
  • 数日以内で見てもらいたい 🐢

参考リンク

スクリーンショット

Before After

Summary by CodeRabbit

  • 新機能

    • アプリに検索機能を追加しました。
    • データベースモジュールを追加して、参加者情報の取得と更新が可能になりました。
  • バグ修正

    • 特定の条件下でアプリがクラッシュする問題を修正しました。
  • ドキュメント

    • ユーザーガイドにデータベース機能の説明を追加しました。
  • リファクタリング

    • 内部コードの整理を行い、パフォーマンスを向上させました。
  • スタイル

    • ユーザーインターフェースのデザインを改善しました。
  • テスト

    • 新しい機能に対するテストケースを追加しました。
  • その他の作業

    • 依存関係の更新とメンテナンスを行いました。
  • リバート

    • 不具合を引き起こす変更を元に戻しました。

@tatsutakein tatsutakein requested a review from a team as a code owner December 11, 2023 10:05
Copy link

coderabbitai bot commented Dec 11, 2023

Walkthrough

アプリケーションのAndroid、iOS、JSプラットフォームにわたるデータベース機能が強化されました。Kotlin Multiplatformプロジェクトにおいて、SQLDelightの設定が追加され、各プラットフォーム用のDriverFactoryが導入されました。Koinを使用した依存性注入が更新され、新しいデータベースモジュールが追加されています。これにより、アプリケーションのデータ管理能力が向上しました。

Changes

ファイル 変更概要
app/android/src/.../MainActivity.kt
app/shared/src/.../NitoApp.kt
app/ios-combined/src/.../KmpEntryPoint.kt
KoinモジュールにContextdatabaseModuleを追加
app/ios/App/Nito/.../project.pbxproj OTHER_LDFLAGSセクションに変更がありますが、公開されているエンティティの宣言には変更がありません。
app/shared/build.gradle.kts projects.core.databaseへの依存関係を追加
build-logic/build.gradle.kts
build-logic/src/main/kotlin/.../KmpSqlDelightPlugin.kt
"kmpSqlDelight"プラグインの登録を追加
core/database/build.gradle.kts nito.primitive.kmp.sqldelightプラグインとSQLDelightの設定を追加
core/database/src/.../Database.android.kt
core/database/src/.../Database.ios.kt
core/database/src/.../Database.js.kt
各プラットフォーム向けのDriverFactoryクラスを追加
core/database/src/.../di/DatabaseModule.*.kt 各プラットフォーム向けのcreateDriverFactory関数を追加
core/database/src/commonMain/kotlin/.../Database.kt
core/database/src/commonMain/kotlin/.../adapter/ParticipantStatus.kt
core/database/src/commonMain/kotlin/.../di/DatabaseModule.kt
core/database/src/commonMain/sqldelight/.../Participants.sq
データベース作成関数、アダプタ、Koinモジュール、SQLクエリを追加

🐰💻

コードの森を駆け巡り
データベースを編み直し
プラグインを植え、依存を結び
新たな機能、芽吹き始めり

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ?


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

@tatsutakein tatsutakein merged commit a45bf0e into main Dec 11, 2023
4 checks passed
@tatsutakein tatsutakein deleted the rt/add-sqldelight branch December 11, 2023 10:08
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d13ab19 and 01531e2.
Files ignored due to filter (1)
  • gradle/libs.versions.toml
Files selected for processing (18)
  • app/android/src/androidMain/kotlin/club/nito/app/MainActivity.kt (3 hunks)
  • app/ios-combined/src/commonMain/kotlin/club/nito/ios/combined/KmpEntryPoint.kt (2 hunks)
  • app/ios/App/Nito/Nito.xcodeproj/project.pbxproj (6 hunks)
  • app/shared/build.gradle.kts (1 hunks)
  • app/shared/src/commonMain/kotlin/club/nito/app/shared/NitoApp.kt (2 hunks)
  • build-logic/build.gradle.kts (1 hunks)
  • build-logic/src/main/kotlin/club/nito/primitive/KmpSqlDelightPlugin.kt (1 hunks)
  • core/database/build.gradle.kts (2 hunks)
  • core/database/src/androidMain/kotlin/club/nito/core/database/Database.android.kt (1 hunks)
  • core/database/src/androidMain/kotlin/club/nito/core/database/di/DatabaseModule.android.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/Database.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/adapter/ParticipantStatus.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/di/DatabaseModule.kt (1 hunks)
  • core/database/src/commonMain/sqldelight/club/nito/core/database/Participants.sq (1 hunks)
  • core/database/src/iosMain/kotlin/club/nito/core/database/Database.ios.kt (1 hunks)
  • core/database/src/iosMain/kotlin/club/nito/core/database/di/DatabaseModule.ios.kt (1 hunks)
  • core/database/src/jsMain/kotlin/club/nito/core/database/Database.js.kt (1 hunks)
  • core/database/src/jsMain/kotlin/club/nito/core/database/di/DatabaseModule.js.kt (1 hunks)
Files skipped from review due to trivial changes (1)
  • app/ios/App/Nito/Nito.xcodeproj/project.pbxproj
Additional comments: 27
app/android/src/androidMain/kotlin/club/nito/app/MainActivity.kt (3)
  • 3-3: インポートされたandroid.content.Contextは、Koinモジュール内でContextのインスタンスを提供するために使用されています。

  • 16-16: インポートされたorg.koin.dsl.moduleは、KoinのDI設定においてモジュールを定義するために使用されています。

  • 42-45: initKoinメソッド内で追加されたKoinモジュールは、アプリケーションのContextをシングルインスタンスとして提供するためのものです。これにより、アプリケーション全体でContextを簡単に注入できるようになります。

app/ios-combined/src/commonMain/kotlin/club/nito/ios/combined/KmpEntryPoint.kt (3)
  • 5-11: databaseModuleのインポートが追加されています。これはSQLDelightの統合に必要な変更であり、適切に行われているようです。

  • 36-42: databaseModuleがKoinのモジュールリストに追加されています。これにより、アプリケーションの依存性注入セットアップにSQLDelightが含まれるようになります。

  • 38-38: コメントアウトされたfakeRemoteDataSourceModuleについて、これが意図的な変更であるかどうかを確認してください。もし意図的でない場合は、削除またはコメントアウトを解除する必要があります。

app/shared/build.gradle.kts (1)
  • 15-21: projects.core.databaseへの依存関係の追加が確認されました。この変更が他のモジュールやプラットフォームに影響を与えないか、また、バージョンの互換性があるかを確認してください。
app/shared/src/commonMain/kotlin/club/nito/app/shared/NitoApp.kt (2)
  • 11-17: databaseModuleのインポートが追加されており、これはSQLDelightの統合に必要な変更であることを確認しました。

  • 46-52: databaseModuleKoinApplicationmodules関数内に追加されています。これにより、SQLDelightのデータベースインスタンスが依存性注入に含まれるようになりました。この変更は、プロジェクトのインフラストラクチャにSQLDelightを統合するというPRの目的に沿っています。

build-logic/build.gradle.kts (1)
  • 65-68: プラグインの登録が正しく行われており、プロジェクトのSQLDelight統合目的に沿っています。プラグインIDと実装クラスがプロジェクトの命名規則に従っており、他のプラグインと一貫性があります。
core/database/build.gradle.kts (3)
  • 3-8: プラグインブロックに nito.primitive.kmp.sqldelight プラグインが追加されています。これはPRの目的であるSQLDelightの統合と一致しています。

  • 15-25: commonMain ソースセットに依存関係が追加されています。これはKotlin Multiplatformプロジェクトに期待される動作です。ただし、追加された依存関係がデータベースモジュールに必要かどうかを確認する必要があります。

  • 29-36: SQLDelightの設定ブロックが追加され、データベースのプロパティを定義しています。データベース名とパッケージ名が指定されており、これはSQLDelightのコード生成に不可欠です。

core/database/src/androidMain/kotlin/club/nito/core/database/Database.android.kt (3)
  • 8-24: DATABASE_NAMEの定義を確認してください。この変数はこのファイル内で定義されていないようですが、他の場所で定義されている必要があります。

  • 17-20: onConfigureメソッド内のenableWriteAheadLoggingsetForeignKeyConstraintsEnabledの呼び出しは、データベースのパフォーマンスと整合性を向上させるための適切な設定です。

  • 8-8: DriverFactoryクラスはinternal修飾子で適切に宣言されており、モジュール内部でのみアクセス可能です。

core/database/src/androidMain/kotlin/club/nito/core/database/di/DatabaseModule.android.kt (1)
  • 6-8: Scope.createDriverFactory関数内でget()を使用してcontextを取得していますが、get()が期待する型のオブジェクトがScopeに存在することを確認してください。また、DriverFactoryのコンストラクタがcontextをどのように使用するかも確認する必要があります。
core/database/src/commonMain/kotlin/club/nito/core/database/Database.kt (3)
  • 6-8: DriverFactory クラスが追加されており、これはプラットフォーム固有の SQL ドライバーを生成するためのものです。各プラットフォームに対して actual 実装が存在することを確認してください。

  • 12-22: createDatabase 関数は DriverFactory を使用してデータベースインスタンスを生成しています。この関数の実装は問題ないように見えますが、EnumColumnAdapter の使用が ParticipantsstatusAdapter に対して適切であることを確認してください。

  • 21-21: コメントに「データベースでさらに作業を行う (下記参照)」とありますが、その詳細が提供されていません。このコメントが指す「下記」の部分がこのファイル内に存在するか、または別の場所にあるのかを確認してください。

core/database/src/commonMain/kotlin/club/nito/core/database/di/DatabaseModule.kt (2)
  • 9-14: databaseModuleの定義は、createDatabase関数をsingleスコープで呼び出しており、依存性注入のセットアップが適切に行われているようです。ただし、createDatabase関数の引数が正しいかどうか、またcreateDriverFactoryが各プラットフォームモジュールで適切に実装されているかどうかを確認する必要があります。

  • 17-17: createDriverFactory関数のexpect宣言は、マルチプラットフォーム対応のために各プラットフォームモジュールでactual実装が提供されていることを期待しています。この実装が存在するかどうかを確認してください。

core/database/src/commonMain/sqldelight/club/nito/core/database/Participants.sq (1)
  • 3-7: SQLDelightテーブル定義において、ParticipantStatusTEXT型として扱うことは、ParticipantStatusが適切にシリアライズされることを前提としています。ParticipantStatusAdapterがこの変換を担当することを確認してください。
core/database/src/iosMain/kotlin/club/nito/core/database/Database.ios.kt (1)
  • 6-10: DATABASE_NAME 定数がこのファイルに定義されていないようです。他の場所で定義されているか確認してください。
core/database/src/iosMain/kotlin/club/nito/core/database/di/DatabaseModule.ios.kt (1)
  • 1-6: iOSプラットフォーム用のDriverFactoryインスタンスを生成するcreateDriverFactory関数の実装が単純であり、特に問題は見受けられません。ただし、DriverFactoryがパラメーターや追加の設定を必要としないかどうかを確認する必要があります。iOS用のDriverFactoryがパラメーターや設定を必要とする場合は、その実装を追加する必要があります。
core/database/src/jsMain/kotlin/club/nito/core/database/Database.js.kt (1)
  • 7-14: DriverFactoryクラスとcreateDriverメソッドの実装は、JSプラットフォーム用のSQLドライバーを生成するためのものであり、WebWorkerDriverを使用しています。ただし、Workerのインスタンス化に使用されているnew URLのパスが正しいかどうかは、レビューからは判断できません。このURLが実際に存在し、適切なリソースを指しているかを確認する必要があります。また、Database.Schema.create(it)alsoブロック内で呼び出されており、これによりドライバーの生成と同時にデータベーススキーマが作成されることが保証されています。
core/database/src/jsMain/kotlin/club/nito/core/database/di/DatabaseModule.js.kt (1)
  • 1-6: この変更は、Kotlin Multiplatform プロジェクトの JS プラットフォーム固有の DriverFactory を生成するための createDriverFactory 関数を適切に実装しています。DriverFactory のコンストラクタがパラメータを必要としないことを確認してください。

import app.cash.sqldelight.ColumnAdapter
import club.nito.core.model.participant.ParticipantStatus

internal data object ParticipantStatusAdapter : ColumnAdapter<ParticipantStatus, String> {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data objectの宣言はKotlinでは無効です。シングルトンパターンを実装するためにはobjectのみを使用する必要があります。

- internal data object ParticipantStatusAdapter : ColumnAdapter<ParticipantStatus, String> {
+ internal object ParticipantStatusAdapter : ColumnAdapter<ParticipantStatus, String> {

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
internal data object ParticipantStatusAdapter : ColumnAdapter<ParticipantStatus, String> {
internal object ParticipantStatusAdapter : ColumnAdapter<ParticipantStatus, String> {

Comment on lines +21 to +37
upsert {
UPDATE participants SET
`schedule_id` = :schedule_id,
`user_id` = :user_id,
`status` = :status
WHERE schedule_id = :schedule_id AND user_id = :user_id;

INSERT OR IGNORE INTO participants (
`schedule_id`,
`user_id`,
`status`
) VALUES (
:schedule_id,
:user_id,
:status
);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

upsert操作は、UPDATEINSERT OR IGNOREの組み合わせで実装されていますが、UPDATEが成功したかどうかに基づいてINSERTをスキップするロジックが見当たりません。SQLDelightではINSERTUPDATEの順序を逆にして、INSERT OR REPLACEを使用することが一般的です。このロジックが意図した通りに動作するか確認してください。

-  UPDATE participants SET
-    `schedule_id` = :schedule_id,
-    `user_id` = :user_id,
-    `status` = :status
-  WHERE schedule_id = :schedule_id AND user_id = :user_id;
-
-  INSERT OR IGNORE INTO participants (
-    `schedule_id`,
-    `user_id`,
-    `status`
-  ) VALUES (
-    :schedule_id,
-    :user_id,
-    :status
-  );
+  INSERT OR REPLACE INTO participants (
+    `schedule_id`,
+    `user_id`,
+    `status`
+  ) VALUES (
+    :schedule_id,
+    :user_id,
+    :status
+  );

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
upsert {
UPDATE participants SET
`schedule_id` = :schedule_id,
`user_id` = :user_id,
`status` = :status
WHERE schedule_id = :schedule_id AND user_id = :user_id;
INSERT OR IGNORE INTO participants (
`schedule_id`,
`user_id`,
`status`
) VALUES (
:schedule_id,
:user_id,
:status
);
}
upsert {
INSERT OR REPLACE INTO participants (
`schedule_id`,
`user_id`,
`status`
) VALUES (
:schedule_id,
:user_id,
:status
);
}

Comment on lines +1 to +47
package club.nito.primitive

import org.gradle.api.Plugin
import org.gradle.api.Project

@Suppress("unused")
class KmpSqlDelightPlugin : Plugin<Project> {
override fun apply(target: Project) {
with(target) {
with(pluginManager) {
apply("app.cash.sqldelight")
}
kotlin {
with(sourceSets) {
getByName("commonMain").apply {
dependencies {
implementation(libs.library("sqldelightPrimitiveAdapters"))
implementation(libs.library("sqldelightCoroutinesExtensions"))
}
}
findByName("androidMain")?.apply {
dependencies {
implementation(libs.library("sqldelightAndroidDriver"))
}
}
findByName("iosMain")?.apply {
dependencies {
implementation(libs.library("sqldelightNativeDriver"))

// Need to explicitly depend on these, otherwise the build fails.
implementation(libs.library("statelyCommon"))
implementation(libs.library("statelyIsolate"))
implementation(libs.library("statelyIsoCollections"))
}
}
findByName("jsMain")?.apply {
dependencies {
implementation(libs.library("sqldelightWebWorkerDriver"))
implementation(npm("sql.js", "1.6.2"))
implementation(devNpm("copy-webpack-plugin", "9.1.0"))
}
}
}
}
}
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

プラグインの実装は適切に行われており、特に問題は見受けられません。ただし、jsMainソースセットにハードコードされたバージョン番号が含まれています。これはメンテナンスの観点から将来的に問題になる可能性があるため、バージョン管理の戦略を再検討することをお勧めします。また、エラーハンドリングやプラグインが正しく適用されていることを確認するための設定チェックを追加することも検討してください。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant