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

✨ Dao を追加 #147

Merged
merged 5 commits into from
Dec 12, 2023
Merged

✨ Dao を追加 #147

merged 5 commits into from
Dec 12, 2023

Conversation

tatsutakein
Copy link
Member

@tatsutakein tatsutakein commented Dec 12, 2023

Issue

  • close #ISSUE_NUMBER 🦕

概要

以下の Dao を追加します。

  • ParticipantDao
  • ProfileDao
  • PlaceDao
  • ScheduleDao

レビュー観点

特になし

レビューレベル

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

レビュー優先度

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

参考リンク

スクリーンショット

Before After

Summary by CodeRabbit

  • 新機能

    • アプリに検索機能を追加しました。
  • ドキュメント

    • ユーザープロフィールのストリームを取得するための新しいDAOインターフェースを追加しました。
    • データベースエンティティからドメインモデルへの変換機能を提供する新しいデータモデルクラスを追加しました。
  • バグ修正

    • テーブルの作成とインデックス作成にIF NOT EXISTS句を追加し、重複作成を防ぎました。
  • リファクタリング

    • データベースアダプターのコードを再編成し、パッケージ構造を整理しました。
  • その他の変更

    • 内部パッケージ構造の変更に伴い、関連するインポート文を更新しました。

@tatsutakein tatsutakein requested a review from a team as a code owner December 12, 2023 00:57
Copy link

coderabbitai bot commented Dec 12, 2023

Walkthrough

データベース関連のファイル群において、新しいアダプターの追加、DAOインターフェースの導入、SQLDelightのスキーマ更新など、多岐にわたる変更が行われました。これらの変更は、Instant型とLong型の変換、参加者や場所、スケジュールのデータ処理の改善を目的としています。

Changes

ファイルパス 変更概要
.../core/database/Database.kt
.../core/database/adapter/InstantLongColumnAdapter.kt
.../core/database/di/DatabaseModule.kt
InstantLongの変換に関連するアダプターの追加とDAOの実装の変更
.../core/database/participant/...
.../core/database/place/...
.../core/database/profile/...
.../core/database/schedule/...
DAOインターフェースとその実装、モデル変換関数の追加
.../core/database/.../*.sq SQLDelightのスキーマとクエリの更新
.../core/domain/...
.../core/model/...
.../core/network/...
.../core/ui/...
.../feature/schedule/...
パッケージ構造の変更とインポートパスの修正

🐰✨
コードの森で変更が舞い、
新しいアダプターが光を放つ。
データが流れる川は、
よりクリアに、より速く。 🌟🌙

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

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: 6

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between a45bf0e and e1bd891.
Files selected for processing (29)
  • core/database/src/commonMain/kotlin/club/nito/core/database/Database.kt (2 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/adapter/InstantLongColumnAdapter.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/di/DatabaseModule.kt (2 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/participant/ParticipantDao.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/participant/SqlDelightParticipantDao.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/participant/adapter/ParticipantStatus.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/participant/model/LocalParticipant.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/place/PlaceDao.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/place/SqlDelightPlaceDao.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/place/model/LocalPlace.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/profile/ProfileDao.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/profile/SqlDelightProfileDao.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/profile/model/LocalUserProfile.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/schedule/ScheduleDao.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/schedule/SqlDelightScheduleDao.kt (1 hunks)
  • core/database/src/commonMain/kotlin/club/nito/core/database/schedule/model/LocalSchedule.kt (1 hunks)
  • core/database/src/commonMain/sqldelight/club/nito/core/database/Participants.sq (2 hunks)
  • core/database/src/commonMain/sqldelight/club/nito/core/database/Places.sq (1 hunks)
  • core/database/src/commonMain/sqldelight/club/nito/core/database/Profiles.sq (1 hunks)
  • core/database/src/commonMain/sqldelight/club/nito/core/database/Schedules.sq (1 hunks)
  • core/domain/src/commonMain/kotlin/club/nito/core/domain/extension/ParticipantExtensions.kt (1 hunks)
  • core/domain/src/commonMain/kotlin/club/nito/core/domain/model/ParticipantSchedule.kt (1 hunks)
  • core/model/src/commonMain/kotlin/club/nito/core/model/participant/ParticipantUser.kt (1 hunks)
  • core/model/src/commonMain/kotlin/club/nito/core/model/place/Place.kt (1 hunks)
  • core/model/src/commonMain/kotlin/club/nito/core/model/schedule/ScheduleWithPlace.kt (1 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/place/model/NetworkPlace.kt (3 hunks)
  • core/ui/src/commonMain/kotlin/club/nito/core/ui/ParticipantScheduleItem.kt (1 hunks)
  • feature/schedule/src/commonMain/kotlin/club/nito/feature/schedule/detail/ScheduleDetailIntent.kt (1 hunks)
  • feature/schedule/src/commonMain/kotlin/club/nito/feature/schedule/detail/ScheduleDetailScreen.kt (1 hunks)
Files skipped from review due to trivial changes (2)
  • core/database/src/commonMain/kotlin/club/nito/core/database/adapter/InstantLongColumnAdapter.kt
  • core/database/src/commonMain/kotlin/club/nito/core/database/place/model/LocalPlace.kt
Additional comments: 40
core/database/src/commonMain/kotlin/club/nito/core/database/Database.kt (2)
  • 5-5: インポート文 import club.nito.core.database.adapter.InstantLongColumnAdapter の追加は、InstantLong 型の変換を扱うための PR の目的に合致しています。この変更に問題はないようです。

  • 20-23: Schedules.Adapter の初期化に scheduled_atAdaptermet_atAdapterInstantLongColumnAdapter を使用して追加されています。Schedules テーブルに対応する scheduled_atmet_at のカラムがこのアダプタと互換性があることを確認することが重要です。


Schedules テーブルに scheduled_atmet_at のカラムが存在し、InstantLongColumnAdapter と互換性があることが確認されました。変更は問題ないと思われます。

core/database/src/commonMain/kotlin/club/nito/core/database/di/DatabaseModule.kt (3)
  • 5-12: 新しいDAOインターフェースとその実装のインポートが追加されています。これらの変更は、プルリクエストの目的に沿っているようです。

  • 26-29: DAOの実装に対するバインディングが正しく追加されています。singleOfbindの使用法が適切で、KoinのDSLに従っています。

  • 22-32: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [19-30]

全体として、DatabaseModule.ktに対する変更は、プルリクエストの目的とAIによって生成された概要に沿っています。レビューレベルLv1に基づいて、明らかな問題は見受けられません。

core/database/src/commonMain/kotlin/club/nito/core/database/participant/SqlDelightParticipantDao.kt (1)
  • 1-63: このSqlDelightParticipantDaoクラスの実装は、必要なインターフェースを適切に実装しており、データベース操作に関するベストプラクティスに従っています。コードは明確で、保守性も高いです。特に問題は見受けられません。
core/database/src/commonMain/kotlin/club/nito/core/database/participant/adapter/ParticipantStatus.kt (2)
  • 1-1: パッケージ宣言が club.nito.core.database.adapter から club.nito.core.database.participant.adapter に変更されています。この変更により、他のファイルでのこのアダプターのインポートパスが変更される可能性があります。プロジェクト内の他のファイルでのインポートパスの更新を確認してください。

パッケージ宣言の変更に関する以前のレビューコメントに基づいて、シェルスクリプトを実行しましたが、出力はありませんでした。これは、ParticipantStatusAdapterが他のファイルで使用されていないか、もしくは既に必要な変更が行われていることを示唆しています。したがって、この変更に関連する問題は見つかりませんでした。

  • 3-4: ParticipantStatusAdapterColumnAdapter を実装していることを確認してください。また、ParticipantStatus enum が他の場所で正しく使用されているかを確認してください。

最終的なレビューコメント:

ParticipantStatus enum とそのアダプターがコードベースの多くの場所で使用されていることが確認されました。ParticipantStatusAdapterColumnAdapter を実装していること、および ParticipantStatus enum が適切に使用されていることを確認してください。

core/database/src/commonMain/kotlin/club/nito/core/database/participant/model/LocalParticipant.kt (1)
  • 3-24: UserProfile のパッケージ構造やプロパティに変更があるかどうかを確認してください。特に、website プロパティが新しく追加されたものである場合、その変更がドメインモデルに適切に反映されているかを検証する必要があります。

UserProfile のパッケージ構造に変更はないことが確認されました。しかし、website プロパティの存在については、スクリプトの結果が不確定であるため、UserProfile クラスの完全な定義を確認する必要があります。

core/database/src/commonMain/kotlin/club/nito/core/database/profile/ProfileDao.kt (1)
  • 1-30: 新しい ProfileDao インターフェースの定義は適切に見えます。メソッドのシグネチャやコメントに問題はないようです。ただし、このインターフェースの実装が正しく行われているか、およびこれらのインターフェースと対話する可能性のある既存のコードが適切に更新されているかを確認することが重要です。
core/database/src/commonMain/kotlin/club/nito/core/database/profile/SqlDelightProfileDao.kt (1)
  • 14-59: コードレビューを行いましたが、SqlDelightProfileDao クラスに関しては、明確な問題点は見つかりませんでした。データベース操作に対してコルーチンを適切に使用しており、ドメインモデルへのマッピングも適切に行われています。また、バッチ操作のアトミック性を保証するためにトランザクションが使用されている点も良いです。ただし、データベース操作中に発生する可能性のあるエラーに対する明示的な処理はコード内には見当たりませんが、これは上流で処理されている可能性があります。パフォーマンスに関する潜在的な問題も現時点では見つかりませんでした。全体として、このクラスはPRの目的に沿っており、レビューレベルLv1の基準を満たしていると考えられます。
core/database/src/commonMain/kotlin/club/nito/core/database/profile/model/LocalUserProfile.kt (1)
  • 6-12: この変換関数 LocalUserProfile.toModel() は、データベースエンティティをドメインモデルに変換するための適切な実装を提供しています。他の部分に問題は見受けられません。
core/database/src/commonMain/kotlin/club/nito/core/database/schedule/ScheduleDao.kt (1)
  • 1-44: インターフェース ScheduleDao の定義は、Kotlin の慣習に従っており、メソッドの命名や構造に問題は見受けられません。コメントも日本語で記述されており、プルリクエストのタイトルの言語と一致しています。このインターフェースが期待通りに機能するかどうかは、実装と統合テストを通じて検証する必要がありますが、コードレビューの観点からは問題ないと思われます。
core/database/src/commonMain/kotlin/club/nito/core/database/schedule/SqlDelightScheduleDao.kt (2)
  • 20-26: コルーチンとフローの使用に関しては問題ないようです。データベースのクエリ結果をドメインモデルにマッピングする処理が適切に行われています。

  • 55-66: upsert 操作がトランザクションでラップされており、これはアトミック性を確保するための良い実践です。

core/database/src/commonMain/kotlin/club/nito/core/database/schedule/model/LocalSchedule.kt (1)
  • 1-39: 変更されたコードについて、ロジックや構文の明らかな問題は見つかりませんでした。LocalScheduleLocalScheduleWithPlacetoModel関数は、ローカルデータベースエンティティをドメインエンティティにマッピングするために適切に実装されているようです。ただし、LocalScheduleWithPlace.toModel関数内でvenuemeetのプロパティが異なる変数名を使用している点に注意してください。これは、SQLDelightのクエリがフラットな構造を返すため、venuemeetのフィールドを区別する必要があるためと思われます。この実装が意図した通りであることを確認してください。
core/database/src/commonMain/sqldelight/club/nito/core/database/Participants.sq (3)
  • 3-9: 新しいparticipantsテーブルとparticipants_pkeyインデックスの作成にIF NOT EXISTS句を追加したことは、テーブルやインデックスが既に存在する場合のエラーを防ぐための良い実践です。

  • 21-34: 新しく追加されたクエリparticipantStatusByUserIdparticipantUsersByScheduleIdparticipantUsersByScheduleIdsが正しく形成されており、ベストプラクティスに従っていることを確認してください。特に、participantUsersByScheduleIdsクエリで:schedule_idを使用してIN句を使う場合は、適切な型のパラメータが渡されることを保証する必要があります。

  • 36-40: upsertブロックが変更され、statusフィールドのみを更新するようになりました。schedule_iduser_idの更新を削除することは意図的なもので、正しいかどうかを確認してください。これらのフィールドはINSERT OR IGNORE部分で既に設定されているため、更新する必要がない場合がありますが、既存のロジックに影響を与えないことを保証する必要があります。

core/database/src/commonMain/sqldelight/club/nito/core/database/Places.sq (3)
  • 1-8: 新しいurlカラムがplacesテーブルに追加されています。この変更は、AIが生成したサマリーに記載されている内容と一致しています。カラムの定義が適切であることを確認してください。

  • 30-53: upsert操作は、名前付きパラメータを使用して、既存の場所を更新するか、存在しない場合は新しい場所を挿入します。この操作が新しいurlカラムを正しく扱っていることを確認してください。

  • 16-28: クエリplacesplacesByIds、およびplaceByIdは、特定の条件なしで、IDのリストによって、または単一のIDによって場所を取得する単純なSELECTステートメントです。新しいスキーマで期待通りに動作するはずです。

core/database/src/commonMain/sqldelight/club/nito/core/database/Profiles.sq (1)
  • 1-46: SQLDelightのテーブル定義とクエリが適切に実装されています。profilesテーブルの作成、インデックスの追加、およびprofileByIdprofileByIdsupsertクエリは、SQLDelightとSQLiteの標準に従っています。特に修正が必要な点は見受けられません。
core/database/src/commonMain/sqldelight/club/nito/core/database/Schedules.sq (3)
  • 3-10: 新しいテーブル schedules のスキーマ定義が正しく行われています。Instant 型を INTEGER として扱うためのアダプターが必要になることを確認してください。

  • 32-50: scheduleWithPlaceschedulesWithPlace クエリは schedules テーブルと places テーブルを venue_idmeet_id で2回結合しています。これにより、スケジュールに関連する場所の情報を取得できます。ただし、places テーブルに venue_idmeet_id が存在することを確認してください。


#!/bin/bash
# Search for the `CREATE TABLE` statement for `places` to verify the existence of `venue_id` and `meet_id`.
rg 'CREATE TABLE IF NOT EXISTS places' -- '*.sq'
  • 52-75: upsert クエリは、schedules テーブルを更新するか、レコードが存在しない場合は新しいレコードを挿入します。このクエリは SQLDelight の標準的な upsert 操作の構文に従っています。
core/domain/src/commonMain/kotlin/club/nito/core/domain/extension/ParticipantExtensions.kt (2)
  • 3-3: 変更されたインポート文は、ParticipantUser クラスの新しいパッケージ構造を反映しています。これは、PRの目的とAIによって生成された概要で言及された ParticipantUser のパッケージ構造の変更と一致しています。

  • 3-3: 新しいパッケージ構造に更新された ParticipantUser クラスの存在を確認してください。


新しいパスで ParticipantUser クラスが見つかりました。変更は正しく行われているようです。

core/domain/src/commonMain/kotlin/club/nito/core/domain/model/ParticipantSchedule.kt (1)
  • 3-3: 新しいインポート文が追加されていますが、ParticipantSchedule クラス内で ParticipantUser が使用されている箇所は見当たりません。このインポートが必要かどうか、または他の変更がこのファイルに含まれているべきかを確認してください。

新しいインポート文 import club.nito.core.model.participant.ParticipantUser は、ParticipantSchedule.kt ファイル内で ParticipantUser クラスが使用されていることを確認しました。他のファイルでも適切に更新されていることが確認できたため、このインポート文は正しいと判断します。プルリクエストの他の変更については、特に問題が指摘されていないため、このレビューでの検証は完了です。

core/model/src/commonMain/kotlin/club/nito/core/model/participant/ParticipantUser.kt (2)
  • 1-1: パッケージ宣言が変更されたことにより、他のファイルでのインポートパスも更新する必要があります。この変更が他のファイルにどのように影響するかを確認してください。

パッケージ宣言の変更に関する以前のコメントに基づいて、実行されたスクリプトはParticipantUser.ktが古いパスを使用してインポートされているインスタンスを見つけることを目的としていましたが、出力はありませんでした。これは、古いパスを使用してParticipantUser.ktをインポートしているファイルが存在しないことを示唆しています。ただし、これはParticipantUser.ktがコードベースの他の場所で使用されていないか、既にインポートが更新されている可能性があります。確実な結論を出すために、再検証を行うことをお勧めします。

  • 3-3: ParticipantStatusのインポート文が削除されていますが、ParticipantUserクラス内でParticipantStatusが使用されています。この型が同じパッケージ内に存在するか、または別の方法で解決されているかを確認してください。

ParticipantStatusParticipantUserと同じパッケージ内に定義されているため、インポート文の削除は適切です。

core/model/src/commonMain/kotlin/club/nito/core/model/place/Place.kt (1)
  • 4-18: 新しいプロパティ url の追加は、AIが生成した概要とPRの目的と一致しています。この変更により、Place オブジェクトを作成または使用するコードの他の部分で更新が必要かどうかを確認する必要があります。

新しい url プロパティの追加は、プルリクエストの目的とAIによって生成された概要と一致しています。検証スクリプトは Place コンストラクタの使用例を検出していませんが、これはコードベースに問題がないことを示唆している可能性があります。ただし、スクリプトの実行に問題があった可能性もあるため、手動での確認をお勧めします。

core/model/src/commonMain/kotlin/club/nito/core/model/schedule/ScheduleWithPlace.kt (1)
  • 6-13: ScheduleWithPlaceクラスの定義について、venuemeetが同じPlace型であることを確認してください。これは意図的な設計であり、異なる場所を表すために使用されていることを確認する必要があります。もし異なる型が必要であれば、適切な型に変更することを検討してください。
core/network/src/commonMain/kotlin/club/nito/core/network/place/model/NetworkPlace.kt (3)
  • 10-13: KDocコメントにurlパラメータが追加されています。これはPlaceデータクラスにurlプロパティを追加するPRの目的と一致しています。

  • 19-19: NetworkPlaceデータクラスにurlプロパティが追加され、toPlaceメソッドで使用されています。これはPlaceデータクラスの更新に合わせてNetworkPlaceクラスを整えるために必要な変更です。

  • 37-37: createFakeNetworkPlace関数にデフォルトのurl値が追加されています。これはNetworkPlaceクラスへの変更と一致しており、テストや開発目的で使用されるモック関数です。

core/ui/src/commonMain/kotlin/club/nito/core/ui/ParticipantScheduleItem.kt (2)
  • 20-21: 変更されたインポートステートメントが正しいかどうかを確認してください。新しいパッケージ構造にfilterIsAttendancetoUserProfileList関数が存在することを検証する必要があります。

  • 17-24: インポート以外に変更されたコードは見当たらないため、この部分の変更は承認されます。

feature/schedule/src/commonMain/kotlin/club/nito/feature/schedule/detail/ScheduleDetailIntent.kt (1)
  • 4-4: パッケージ構造の変更に伴い、ParticipantUser クラスのインポート文が更新されました。この変更が他のファイルにも適切に反映されているか確認してください。

ParticipantUser クラスのインポート文が新しいパッケージ構造に沿って適切に更新されていることを確認しました。古いインポート文はコードベース内で使用されていないため、この変更は問題ないと判断します。

feature/schedule/src/commonMain/kotlin/club/nito/feature/schedule/detail/ScheduleDetailScreen.kt (1)
  • 57-57: パッケージ構造の変更に伴い、ParticipantUserのインポート文が更新されています。この変更は、PRの目的とAIによって生成された概要に記載されている内容と一致しています。

Comment on lines +16 to +21
/**
* 該当の予定の参加情報のストリームを取得する
*
* @param scheduleId 参加情報を取得するスケジュールID
*/
public fun participantStatusStream(scheduleId: String, userId: String): Flow<ParticipantStatus>
Copy link

Choose a reason for hiding this comment

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

メソッド participantUsersStreamparticipantStatusStream のコメントが同じですが、これらのメソッドが異なる機能を提供していることを考慮すると、コメントを区別することをお勧めします。例えば、participantStatusStream のコメントにはユーザーIDに関する情報を追加することができます。

package club.nito.core.database.profile.model

import club.nito.core.model.UserProfile
import club.nito.core.database.Profiles as LocalUserProfile
Copy link

Choose a reason for hiding this comment

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

エイリアス LocalUserProfile は、インポートされたクラスと同じ名前を使用しています。これは混乱を招く可能性があるため、異なるエイリアス名を使用するか、エイリアスを使用しないことを検討してください。

- import club.nito.core.database.Profiles as LocalUserProfile
+ import club.nito.core.database.Profiles

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
import club.nito.core.database.Profiles as LocalUserProfile
import club.nito.core.database.Profiles

Comment on lines 1 to 35
package club.nito.core.database.place

import club.nito.core.model.place.Place
import kotlinx.coroutines.flow.Flow

public interface PlaceDao {
/**
* 場所一覧のストリームを取得する
*/
public val placesStream: Flow<List<Place>>

/**
* 場所のストリームを取得する
*
* @param placeId 取得する場所の ID
*/
public fun placeStream(placeId: String): Flow<Place?>

/**
* 場所一覧のストリームを取得する
*
* @param placeIds 取得する場所の ID 配列
*/
public fun placesStream(placeIds: List<String>): Flow<List<Place>>

/**
* スケジュール一覧を登録 / 更新する
*/
public fun upsert(entities: List<Place>)

/**
* スケジュールを登録 / 更新する
*/
public fun upsert(entity: Place)
}
Copy link

Choose a reason for hiding this comment

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

インターフェース PlaceDao の実装は、場所に関連するデータアクセスのためのメソッドを提供しています。これには、場所のストリームを取得するメソッドや、場所を登録/更新するメソッドが含まれています。コードは明確で、ドキュメントコメントも適切に提供されています。ただし、upsert メソッドのコメントには「スケジュール」という言葉が使われていますが、これは「場所」の間違いではないでしょうか。これは、コードの意図とドキュメントの整合性を保つために重要です。

public fun placesStream(placeIds: List<String>): Flow<List<Place>>

/**
* スケジュール一覧を登録 / 更新する
Copy link

Choose a reason for hiding this comment

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

upsert メソッドのコメントに「スケジュール」と記載されていますが、これは「場所」の操作を行うメソッドであるため、「場所」に修正する必要があります。これはドキュメントの誤りであり、混乱を招く可能性があります。

-     * スケジュール一覧を登録 / 更新する
+     * 場所一覧を登録 / 更新する
-     * スケジュールを登録 / 更新する
+     * 場所を登録 / 更新する

Also applies to: 32-32

}
}

private fun upsertUserProfile(entity: Place) {
Copy link

Choose a reason for hiding this comment

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

メソッド名 upsertUserProfilePlace エンティティを扱っているにも関わらず、UserProfile という名前になっています。これはおそらくコピペのミスか見落としでしょう。エンティティに合わせて名前を変更することをお勧めします。

- private fun upsertUserProfile(entity: Place) {
+ private fun upsertPlace(entity: Place) {

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
private fun upsertUserProfile(entity: Place) {
private fun upsertPlace(entity: Place) {

Comment on lines 16 to 79
internal class SqlDelightScheduleDao(
private val database: Database,
private val dispatchers: NitoCoroutineDispatchers,
) : ScheduleDao {
override val schedulesStream: Flow<List<Schedule>>
get() = database.schedulesQueries.schedules()
.asFlow()
.mapToList(dispatchers.io)
.map {
it.map(Schedules::toModel)
}

override fun scheduleStream(scheduleId: ScheduleId): Flow<Schedule?> {
return database.schedulesQueries.scheduleById(scheduleId)
.asFlow()
.mapToOneOrNull(dispatchers.io)
.map {
it?.toModel()
}
}

override fun scheduleWithPlaceStream(scheduleId: ScheduleId): Flow<ScheduleWithPlace?> {
return database.schedulesQueries.scheduleWithPlace(scheduleId)
.asFlow()
.mapToOneOrNull(dispatchers.io)
.map {
it?.toModel()
}
}

override fun schedulesStream(scheduleIds: List<ScheduleId>): Flow<List<Schedule>> {
return database.schedulesQueries.schedulesByIds(scheduleIds)
.asFlow()
.mapToList(dispatchers.io)
.map {
it.map(Schedules::toModel)
}
}

override fun upsert(entities: List<Schedule>) {
database.transaction {
for (entity in entities) {
upsertUserProfile(entity)
}
}
}

override fun upsert(entity: Schedule) {
database.transaction {
upsertUserProfile(entity)
}
}

private fun upsertUserProfile(entity: Schedule) {
database.schedulesQueries.upsert(
id = entity.id,
scheduled_at = entity.scheduledAt,
met_at = entity.metAt,
venue_id = entity.venueId,
meet_id = entity.meetId,
description = entity.description,
)
}
}
Copy link

Choose a reason for hiding this comment

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

コードレビューを行いましたが、以下の点について確認が必要です。

  • upsertUserProfile メソッドの名前が upsert という操作を行っていることを示していますが、実際には Schedule エンティティに対して操作を行っています。メソッド名が誤解を招く可能性があるため、upsertSchedule などのより適切な名前に変更することをお勧めします。
  • upsert メソッドと upsertUserProfile メソッドの間でコードの重複が見られます。upsert メソッドは単一の Schedule エンティティを受け取り、upsertUserProfile を呼び出していますが、upsert メソッドはリストを受け取るバージョンと同じように、トランザクションの中で直接 database.schedulesQueries.upsert を呼び出すこともできます。これにより、コードの重複を減らし、メンテナンス性を向上させることができます。
  • upsert メソッドのリストを受け取るバージョンでは、entity の各要素に対して upsertUserProfile を呼び出していますが、このメソッド名が実際の操作内容と一致していないため、混乱を招く可能性があります。
- private fun upsertUserProfile(entity: Schedule) {
+ private fun upsertSchedule(entity: Schedule) {
    database.schedulesQueries.upsert(
        id = entity.id,
        scheduled_at = entity.scheduledAt,
        met_at = entity.metAt,
        venue_id = entity.venueId,
        meet_id = entity.meetId,
        description = entity.description,
    )
}

- override fun upsert(entity: Schedule) {
-     database.transaction {
-         upsertUserProfile(entity)
-     }
- }
+ override fun upsert(entity: Schedule) = upsert(listOf(entity))

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 class SqlDelightScheduleDao(
private val database: Database,
private val dispatchers: NitoCoroutineDispatchers,
) : ScheduleDao {
override val schedulesStream: Flow<List<Schedule>>
get() = database.schedulesQueries.schedules()
.asFlow()
.mapToList(dispatchers.io)
.map {
it.map(Schedules::toModel)
}
override fun scheduleStream(scheduleId: ScheduleId): Flow<Schedule?> {
return database.schedulesQueries.scheduleById(scheduleId)
.asFlow()
.mapToOneOrNull(dispatchers.io)
.map {
it?.toModel()
}
}
override fun scheduleWithPlaceStream(scheduleId: ScheduleId): Flow<ScheduleWithPlace?> {
return database.schedulesQueries.scheduleWithPlace(scheduleId)
.asFlow()
.mapToOneOrNull(dispatchers.io)
.map {
it?.toModel()
}
}
override fun schedulesStream(scheduleIds: List<ScheduleId>): Flow<List<Schedule>> {
return database.schedulesQueries.schedulesByIds(scheduleIds)
.asFlow()
.mapToList(dispatchers.io)
.map {
it.map(Schedules::toModel)
}
}
override fun upsert(entities: List<Schedule>) {
database.transaction {
for (entity in entities) {
upsertUserProfile(entity)
}
}
}
override fun upsert(entity: Schedule) {
database.transaction {
upsertUserProfile(entity)
}
}
private fun upsertUserProfile(entity: Schedule) {
database.schedulesQueries.upsert(
id = entity.id,
scheduled_at = entity.scheduledAt,
met_at = entity.metAt,
venue_id = entity.venueId,
meet_id = entity.meetId,
description = entity.description,
)
}
}
internal class SqlDelightScheduleDao(
private val database: Database,
private val dispatchers: NitoCoroutineDispatchers,
) : ScheduleDao {
override val schedulesStream: Flow<List<Schedule>>
get() = database.schedulesQueries.schedules()
.asFlow()
.mapToList(dispatchers.io)
.map {
it.map(Schedules::toModel)
}
override fun scheduleStream(scheduleId: ScheduleId): Flow<Schedule?> {
return database.schedulesQueries.scheduleById(scheduleId)
.asFlow()
.mapToOneOrNull(dispatchers.io)
.map {
it?.toModel()
}
}
override fun scheduleWithPlaceStream(scheduleId: ScheduleId): Flow<ScheduleWithPlace?> {
return database.schedulesQueries.scheduleWithPlace(scheduleId)
.asFlow()
.mapToOneOrNull(dispatchers.io)
.map {
it?.toModel()
}
}
override fun schedulesStream(scheduleIds: List<ScheduleId>): Flow<List<Schedule>> {
return database.schedulesQueries.schedulesByIds(scheduleIds)
.asFlow()
.mapToList(dispatchers.io)
.map {
it.map(Schedules::toModel)
}
}
override fun upsert(entities: List<Schedule>) {
database.transaction {
for (entity in entities) {
upsertSchedule(entity)
}
}
}
override fun upsert(entity: Schedule) = upsert(listOf(entity))
private fun upsertSchedule(entity: Schedule) {
database.schedulesQueries.upsert(
id = entity.id,
scheduled_at = entity.scheduledAt,
met_at = entity.metAt,
venue_id = entity.venueId,
meet_id = entity.meetId,
description = entity.description,
)
}
}

@tatsutakein tatsutakein merged commit 081000c into main Dec 12, 2023
3 checks passed
@tatsutakein tatsutakein deleted the rt/add-daos branch December 12, 2023 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant