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

🐛 強制リフレッシュ処理を追加 #113

Merged
merged 1 commit into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
package club.nito.core.data

import club.nito.core.datastore.DataStore
import club.nito.core.model.AuthStatus
import club.nito.core.model.UserInfo
import club.nito.core.network.auth.AuthRemoteDataSource
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map

public class DefaultAuthRepository(
private val remoteDataSource: AuthRemoteDataSource,
private val dataStore: DataStore,
) : AuthRepository {
override val authStatus: Flow<AuthStatus> = remoteDataSource.authStatus
override val authStatus: Flow<AuthStatus> = remoteDataSource.authStatus.map {
if (it is AuthStatus.Authenticated && dataStore.isRefreshed().not()) {
remoteDataSource.refreshCurrentSession()
dataStore.setRefreshed(true)
}

it
}
Comment on lines +14 to +21
Copy link

Choose a reason for hiding this comment

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

authStatusフローの変換ロジックに例外処理が含まれていません。remoteDataSource.refreshCurrentSession()の呼び出しに失敗した場合に備えて、エラーハンドリングを追加することを検討してください。また、セッションのリフレッシュが成功した後に新しいAuthStatusを発行する必要があるかどうかを確認してください。

+           try {
+           } catch (e: Exception) {
+               // Handle the exception, possibly by logging or rethrowing
+           }

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
override val authStatus: Flow<AuthStatus> = remoteDataSource.authStatus.map {
if (it is AuthStatus.Authenticated && dataStore.isRefreshed().not()) {
remoteDataSource.refreshCurrentSession()
dataStore.setRefreshed(true)
}
it
}
override val authStatus: Flow<AuthStatus> = remoteDataSource.authStatus.map {
if (it is AuthStatus.Authenticated && dataStore.isRefreshed().not()) {
try {
remoteDataSource.refreshCurrentSession()
dataStore.setRefreshed(true)
} catch (e: Exception) {
// Handle the exception, possibly by logging or rethrowing
}
}
it
}


override suspend fun login(email: String, password: String): Unit = remoteDataSource.login(
email = email,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
package club.nito.core.datastore

public sealed interface DataStore
public sealed interface DataStore {
public suspend fun setRefreshed(isRefreshed: Boolean)
public suspend fun isRefreshed(): Boolean
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,20 @@ import com.russhwolf.settings.coroutines.SuspendSettings
import com.russhwolf.settings.coroutines.toSuspendSettings
import kotlinx.serialization.json.Json

@OptIn(ExperimentalSettingsApi::class)
public class SettingsDataStore(
private val settings: Settings,
@OptIn(ExperimentalSettingsApi::class)
private val suspendSettings: SuspendSettings = settings.toSuspendSettings(),
private val json: Json,
) : DataStore
) : DataStore {
override suspend fun setRefreshed(isRefreshed: Boolean) {
suspendSettings.putBoolean(REFRESHED_KEY, isRefreshed)
}

override suspend fun isRefreshed(): Boolean = suspendSettings.getBoolean(REFRESHED_KEY, false)

public companion object {
private const val REFRESHED_KEY = "refreshed-v0-3-3"
}
}
Comment on lines +9 to +25
Copy link

Choose a reason for hiding this comment

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

💡 NOTE
Codebase verification is a beta feature.

SettingsDataStore クラスに対する変更は、DataStore インターフェースの実装と実験的な SuspendSettings API の使用を含んでいます。@OptIn(ExperimentalSettingsApi::class) アノテーションは、他の2つのファイルでも使用されていることが確認されましたが、DataStore インターフェースの setRefreshed および isRefreshed メソッドを実装している他のクラスは見つかりませんでした。このことは、新しいインターフェースメソッドが現在のコードベースで広く採用されていない可能性があることを示唆しています。この変更が他の部分にどのような影響を与えるかを検証する必要があります。

Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ public sealed interface AuthRemoteDataSource {
public suspend fun logout()
public suspend fun modifyAuthUser(email: String?, password: String?): UserInfo
public suspend fun authIfNeeded()
public suspend fun refreshCurrentSession()
}
Comment on lines 11 to 15
Copy link

Choose a reason for hiding this comment

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

💡 NOTE
Codebase verification is a beta feature.

AuthRemoteDataSourceインターフェースにrefreshCurrentSessionメソッドが追加されましたが、このインターフェースを実装するクラスに新しいメソッドの実装が見つかりませんでした。すべての実装クラスが新しいメソッドを適切に実装しているか確認する必要があります。

Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ public class FakeAuthRemoteDataSource(
// Do nothing
}

override suspend fun refreshCurrentSession() {
// Do nothing
}

private fun createFakeUserInfo(
aud: String = "aud",
confirmationSentAt: Instant? = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,6 @@ public class SupabaseAuthRemoteDataSource(
goTrue.refreshCurrentSession()
}
}

override suspend fun refreshCurrentSession(): Unit = goTrue.refreshCurrentSession()
}