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

👍 Interceptor を追加 #120

Merged
merged 3 commits into from
Dec 5, 2023
Merged

👍 Interceptor を追加 #120

merged 3 commits into from
Dec 5, 2023

Conversation

tatsutakein
Copy link
Member

@tatsutakein tatsutakein commented Dec 5, 2023

Issue

  • close #ISSUE_NUMBER 🦕

概要

Interceptor を追加します。

レビュー観点

特になし

レビューレベル

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

レビュー優先度

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

参考リンク

スクリーンショット

Before After

Summary by CodeRabbit

  • 新機能

    • アプリケーションのビルド設定情報を提供するBuildConfigインターフェースを追加しました。
    • 検索機能をアプリに追加しました。
  • バグ修正

    • 初期化プロセスに関連するinitKoinブロックをMainActivityに追加しました。
  • ドキュメント

    • iOSアプリの初期化ロジックにbuildConfigパラメータを導入しました。
  • リファクタ

    • NitoApp関数にinitKoinパラメータを追加し、パラメータの順序を変更しました。
  • スタイル

    • 検索バーのスタイルをstyles.cssに追加しました。
  • テスト

    • 各プラットフォーム向けにHTTPクライアントエンジンを作成するcreateHttpEngine関数を追加しました。
  • チョア

    • 依存関係注入フレームワークKoinのモジュールを追加しました。
  • リバート

    • 特になし。

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

coderabbitai bot commented Dec 5, 2023

Walkthrough

ビルド設定に関する変更がアプリケーション全体にわたって行われました。新しいBuildConfigインターフェースが導入され、バージョン名やデバッグビルドのフラグを提供します。KotlinとSwiftの両方のコードベースで、依存性注入フレームワークKoinを使用してBuildConfigのインスタンスを提供するように変更されています。また、HTTPクライアントエンジンの作成にBuildConfigが使用されるようになりました。

Changes

ファイルパス 変更概要
app/android/.../AppModule.kt
app/ios-combined/.../KmpEntryPoint.kt
app/ios/Modules/Sources/KmpContainer/Container.swift
core/model/.../BuildConfig.kt
BuildConfigインターフェースの追加と、それに基づく依存性注入の設定
app/android/.../MainActivity.kt
app/shared/.../NitoApp.kt
Koinの初期化ブロックの追加と変更
core/network/.../SupabaseClient.*.kt
core/network/.../di/SupabaseClientModule.kt
BuildConfigを使用したHTTPクライアントエンジンの作成関数の追加

🐰✨
コードの森を駆けるうさぎ、
新しい設定を耳に乗せ、
依存性の糸を編み直し、
ビルドの夢を紡ぎ出す。


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

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 5c2bddc and 75f0add.
Files selected for processing (11)
  • app/android/src/androidMain/kotlin/club/nito/app/AppModule.kt (1 hunks)
  • app/android/src/androidMain/kotlin/club/nito/app/MainActivity.kt (1 hunks)
  • app/ios-combined/src/commonMain/kotlin/club/nito/ios/combined/KmpEntryPoint.kt (2 hunks)
  • app/ios/Modules/Sources/KmpContainer/Container.swift (2 hunks)
  • app/shared/src/commonMain/kotlin/club/nito/app/shared/NitoApp.kt (2 hunks)
  • core/model/src/commonMain/kotlin/club/nito/core/model/BuildConfig.kt (1 hunks)
  • core/network/src/androidMain/kotlin/club/nito/core/network/SupabaseClient.android.kt (1 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/SupabaseClient.kt (3 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/di/SupabaseClientModule.kt (2 hunks)
  • core/network/src/iosMain/kotlin/club/nito/core/network/SupabaseClient.ios.kt (1 hunks)
  • core/network/src/jsMain/kotlin/club/nito/core/network/SupabaseClient.js.kt (1 hunks)
Additional comments: 17
app/android/src/androidMain/kotlin/club/nito/app/AppModule.kt (1)
  • 7-13: このコードの変更は、BuildConfigのシングルトンインスタンスをappModuleを通じて提供しており、AppBuildConfigからのプロパティを使用しています。問題は見受けられません。
app/android/src/androidMain/kotlin/club/nito/app/MainActivity.kt (1)
  • 36-44: MainActivitysetContentブロック内にinitKoinブロックが追加されました。これにより、Koinの依存性注入フレームワークの初期化プロセスに変更が加えられたことが示唆されます。appModuleを使用してKoinを初期化することで、アプリケーションの依存関係グラフと制御フローに影響を与える可能性があります。この変更は、プルリクエストの目的に沿っており、レビューで特に指摘する問題は見受けられません。
app/ios-combined/src/commonMain/kotlin/club/nito/ios/combined/KmpEntryPoint.kt (2)
  • 23-24: KmpEntryPointクラスのinit関数にデフォルト値BuildConfig.Emptyを持つ新しいパラメータbuildConfigが追加されました。この変更は、PRの目的と提供されたサマリーに沿っています。

  • 45-46: startKoinブロックにBuildConfigのシングルトンインスタンスを提供する新しいモジュールが追加されました。この変更もPRの目的と提供されたサマリーに一致しています。

app/ios/Modules/Sources/KmpContainer/Container.swift (2)
  • 6-13: 新しい buildConfig パラメーターを doInit メソッドに導入する変更は、PRの目的に合致しています。AppBuildConfig インスタンスを作成して、ビルド設定に基づいてインターセプターを設定するために使用されます。

  • 24-37: AppBuildConfig クラスは BuildConfig に準拠しており、コンパイルフラグに基づいて debugBuild を設定し、アプリのバンドル情報から versionName を設定します。この変更は、ビルド設定をアプリケーションの他の部分で利用可能にするために重要です。

app/shared/src/commonMain/kotlin/club/nito/app/shared/NitoApp.kt (3)
  • 28-31: 新しいパラメータ initKoinNitoApp 関数に追加されました。これにより、Koinの初期化をカスタマイズできるようになります。この変更が他のコンポーザブル関数の呼び出しに影響を与えないことを確認してください。

  • 55-55: initKoin 関数が KoinApplication ブロック内で呼び出されています。これは、Koinの設定をカスタマイズするための適切な場所です。

  • 29-30: shouldKeepOnScreen パラメータの順序が変更されています。これにより、関数のシグネチャが変更されたため、この関数を使用する既存のコードが影響を受ける可能性があります。この変更が意図的であること、および必要なすべての呼び出し場所が更新されていることを確認してください。

core/network/src/androidMain/kotlin/club/nito/core/network/SupabaseClient.android.kt (1)
  • 8-24: この実装は、デバッグビルドかどうかに基づいてログレベルを設定するために buildConfig パラメータを適切に使用しています。これにより、デバッグビルドの場合にはHTTPリクエストとレスポンスの内容がログに記録され、リリースビルドの場合にはログが無効になります。
core/network/src/commonMain/kotlin/club/nito/core/network/SupabaseClient.kt (4)
  • 3-4: BuildConfigのインポートが追加されています。これは、HTTPクライアントの設定に関連する変更を示している可能性があります。

  • 21-24: createNitoSupabaseClient関数にhttpClientEngineパラメータが追加されましたが、このパラメータはnull許容型です。nullを許容することの意図された使用ケースを確認してください。

  • 41-41: createHttpEngine関数がexpectキーワードで宣言されています。これはKotlin Multiplatformプロジェクトにおいて適切です。

  • 43-45: NitoSettingsSessionManagerクラスはprivateであり、Settingsを使用したセッション管理のために正しく実装されているようです。

core/network/src/commonMain/kotlin/club/nito/core/network/di/SupabaseClientModule.kt (2)
  • 3-4: 新しいインポート文 import club.nito.core.network.createHttpEngine が追加されました。これは、HTTPクライアントエンジンを作成するために必要です。

  • 13-21: supabaseClientModule に新しい引数 httpClientEngine が追加され、createHttpEngine(buildConfig = get()) を呼び出して作成されています。これにより、ビルド設定に基づいてHTTPクライアントエンジンが設定されます。

core/network/src/jsMain/kotlin/club/nito/core/network/SupabaseClient.js.kt (1)
  • 6-6: この createHttpEngine 関数は、JavaScript環境用に null を返しています。これは意図的なのか、それとも実装が未完了なのかを確認してください。JavaScript環境ではインターセプターや特定のHTTPクライアントエンジンが適用されない、または必要ない場合がありますが、PRの説明やコードベースのドキュメントでこの挙動が意図されていることを確認することが重要です。

Comment on lines +25 to +28
private object EmptyBuildConfig : BuildConfig {
override val versionName: String = ""
override val debugBuild: Boolean = false
}
Copy link

Choose a reason for hiding this comment

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

EmptyBuildConfigprivateとして宣言されていますが、これは他のモジュールからアクセスできないことを意味します。BuildConfigのデフォルト実装として使用する場合は、publicに変更する必要があります。

- private object EmptyBuildConfig : BuildConfig {
+ public object EmptyBuildConfig : BuildConfig {

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 object EmptyBuildConfig : BuildConfig {
override val versionName: String = ""
override val debugBuild: Boolean = false
}
public object EmptyBuildConfig : BuildConfig {
override val versionName: String = ""
override val debugBuild: Boolean = false
}

import club.nito.core.model.BuildConfig
import io.ktor.client.engine.HttpClientEngine

internal actual fun createHttpEngine(buildConfig: BuildConfig): HttpClientEngine? = null
Copy link

Choose a reason for hiding this comment

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

iOSのcreateHttpEngine関数がnullを返していますが、これは意図した動作ではない可能性があります。サマリーによると、この関数はBuildConfigdebugBuildフラグに基づいてロギング用のインターセプターを設定してHttpClientEngineを作成する必要があります。適切なHttpClientEngineを設定して返すように実装を見直すことをお勧めします。

import kotlinx.serialization.encodeToString
import kotlinx.serialization.json.Json

internal fun createNitoSupabaseClient(
httpClientEngine: HttpClientEngine?,
json: Json,
): SupabaseClient = createSupabaseClient(
Copy link

Choose a reason for hiding this comment

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

SupabaseのURLとキーがハードコードされています。キーが機密情報である場合、セキュリティ上の懸念があります。

@tatsutakein tatsutakein merged commit d457ec0 into main Dec 5, 2023
4 checks passed
@tatsutakein tatsutakein deleted the rt/add-logginginterceptor branch December 5, 2023 11:24
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