Skip to content
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
Expand Up @@ -29,4 +29,6 @@ interface UserRepository {
suspend fun setLastNotificationSyncedEnabled(isEnabled: Boolean)

suspend fun updateNotificationSettings(notificationEnabled: Boolean): Result<UserProfileModel>

suspend fun resetNotificationData()
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.ninecraft.booket.core.data.impl.di

import com.google.firebase.Firebase
import com.google.firebase.installations.FirebaseInstallations
import com.google.firebase.installations.installations
import com.google.firebase.messaging.FirebaseMessaging
import com.google.firebase.messaging.messaging
import com.google.firebase.remoteconfig.FirebaseRemoteConfig
Expand Down Expand Up @@ -32,4 +34,8 @@ internal object FirebaseModule {
@Singleton
@Provides
fun provideFirebaseMessaging(): FirebaseMessaging = Firebase.messaging

@Singleton
@Provides
fun provideFirebaseInstallation(): FirebaseInstallations = Firebase.installations
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package com.ninecraft.booket.core.data.impl.repository

import com.ninecraft.booket.core.common.utils.runSuspendCatching
import com.ninecraft.booket.core.data.api.repository.AuthRepository
import com.ninecraft.booket.core.datastore.api.datasource.NotificationDataSource
import com.ninecraft.booket.core.datastore.api.datasource.TokenDataSource
import com.ninecraft.booket.core.model.AutoLoginState
import com.ninecraft.booket.core.model.UserState
Expand All @@ -16,7 +15,6 @@ private const val KAKAO_PROVIDER_TYPE = "KAKAO"
internal class DefaultAuthRepository @Inject constructor(
private val service: ReedService,
private val tokenDataSource: TokenDataSource,
private val notificationDataSource: NotificationDataSource,
) : AuthRepository {
override suspend fun login(accessToken: String) = runSuspendCatching {
val response = service.login(
Expand All @@ -31,7 +29,6 @@ internal class DefaultAuthRepository @Inject constructor(
override suspend fun logout() = runSuspendCatching {
service.logout()
clearTokens()
clearNotificationDataStore()
}

override suspend fun withdraw() = runSuspendCatching {
Expand Down Expand Up @@ -64,8 +61,4 @@ internal class DefaultAuthRepository @Inject constructor(
val accessToken = tokenDataSource.getAccessToken()
return if (accessToken.isBlank()) UserState.Guest else UserState.LoggedIn
}

private suspend fun clearNotificationDataStore() {
notificationDataSource.clearNotificationDataStore()
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package com.ninecraft.booket.core.data.impl.repository

import com.google.firebase.installations.FirebaseInstallations
import com.google.firebase.messaging.FirebaseMessaging
import com.ninecraft.booket.core.common.utils.runSuspendCatching
import com.ninecraft.booket.core.data.api.repository.UserRepository
import com.ninecraft.booket.core.data.impl.mapper.toModel
import com.ninecraft.booket.core.datastore.api.datasource.NotificationDataSource
import com.ninecraft.booket.core.datastore.api.datasource.OnboardingDataSource
import com.ninecraft.booket.core.network.request.FcmTokenRequest
import com.ninecraft.booket.core.network.request.DeviceRegistrationRequest
import com.ninecraft.booket.core.network.request.NotificationSettingsRequest
import com.ninecraft.booket.core.network.request.TermsAgreementRequest
import com.ninecraft.booket.core.network.service.ReedService
Expand All @@ -21,6 +22,7 @@ internal class DefaultUserRepository @Inject constructor(
private val onboardingDataSource: OnboardingDataSource,
private val notificationDataSource: NotificationDataSource,
private val firebaseMessaging: FirebaseMessaging,
private val firebaseInstallations: FirebaseInstallations,
) : UserRepository {
override suspend fun agreeTerms(termsAgreed: Boolean) = runSuspendCatching {
service.agreeTerms(TermsAgreementRequest(termsAgreed)).toModel()
Expand All @@ -45,12 +47,12 @@ internal class DefaultUserRepository @Inject constructor(
return@runSuspendCatching
}

updateFcmToken(newToken)
registerDevice(newToken)
setFcmToken(newToken)
}

override suspend fun syncFcmToken(fcmToken: String): Result<Unit> = runSuspendCatching {
updateFcmToken(fcmToken)
registerDevice(fcmToken)
setFcmToken(fcmToken)
}

Expand All @@ -73,6 +75,15 @@ internal class DefaultUserRepository @Inject constructor(
service.updateNotificationSettings(NotificationSettingsRequest(notificationEnabled)).toModel()
}

override suspend fun resetNotificationData() {
try {
deleteRemoteFcmToken()
clearNotificationDataStore()
} catch (e: Exception) {
Logger.e("Failed to reset notification data: ${e.message}")
}
}
Comment on lines +78 to +85
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

에러 처리 일관성 및 부분 실패 처리 검토 필요

다음 사항들을 확인해 주세요:

  1. 에러 처리 불일치: 이 메서드는 예외를 로깅만 하고 삼키는 반면, getRemoteFcmToken(), getDeviceId() 등 다른 메서드들은 예외를 다시 던집니다.

  2. 부분 실패 케이스: deleteRemoteFcmToken()은 성공했지만 clearNotificationDataStore()가 실패하거나 그 반대의 경우, 부분적인 정리만 수행됩니다.

현재 구현이 "최선의 노력(best effort)" 정리를 의도한 것이라면:

  • 주석으로 명확히 문서화하거나
  • 두 작업을 독립적으로 try-catch하여 하나가 실패해도 다른 하나는 시도하도록 개선할 수 있습니다.

다음과 같이 개선할 수 있습니다:

 override suspend fun resetNotificationData() {
-        try {
-            deleteRemoteFcmToken()
-            clearNotificationDataStore()
-        } catch (e: Exception) {
-            Logger.e("Failed to reset notification data: ${e.message}")
-        }
+        // 각 정리 작업을 독립적으로 시도하여 하나가 실패해도 다른 작업은 수행
+        runCatching {
+            deleteRemoteFcmToken()
+        }.onFailure { e ->
+            Logger.e("Failed to delete remote FCM token: ${e.message}")
+        }
+        
+        runCatching {
+            clearNotificationDataStore()
+        }.onFailure { e ->
+            Logger.e("Failed to clear notification datastore: ${e.message}")
+        }
     }
🤖 Prompt for AI Agents
In
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt
around lines 78 to 85, the resetNotificationData method swallows exceptions and
performs two operations in one try block which leads to inconsistent error
handling compared to other methods and allows partial failures; either (A) make
this method consistent by rethrowing exceptions after logging so callers can
handle failures, or (B) implement best-effort semantics explicitly: add a brief
comment stating "best-effort cleanup", wrap deleteRemoteFcmToken() and
clearNotificationDataStore() in separate try-catch blocks that log each failure
(including exception details) but ensure the other operation still runs, and do
not swallow errors silently (consider returning a boolean or result object if
callers need status).


private suspend fun getRemoteFcmToken(): String {
return try {
firebaseMessaging.token.await()
Expand All @@ -88,7 +99,25 @@ internal class DefaultUserRepository @Inject constructor(
notificationDataSource.setFcmToken(fcmToken)
}

private suspend fun updateFcmToken(fcmToken: String) {
service.updateFcmToken(FcmTokenRequest(fcmToken))
private suspend fun getDeviceId(): String {
return try {
firebaseInstallations.id.await()
} catch (e: Exception) {
Logger.e("Failed to fetch device ID: ${e.message}")
throw e
}
}

private suspend fun registerDevice(fcmToken: String) {
val deviceId = getDeviceId()
service.upsertDevice(DeviceRegistrationRequest(deviceId, fcmToken))
}

private suspend fun deleteRemoteFcmToken() {
firebaseMessaging.deleteToken().await()
}

private suspend fun clearNotificationDataStore() {
notificationDataSource.clearNotificationDataStore()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

@Serializable
data class FcmTokenRequest(
data class DeviceRegistrationRequest(
@SerialName("deviceId")
val deviceId: String,
@SerialName("fcmToken")
val fcmToken: String,
)
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.ninecraft.booket.core.network.service

import com.ninecraft.booket.core.network.request.BookUpsertRequest
import com.ninecraft.booket.core.network.request.FcmTokenRequest
import com.ninecraft.booket.core.network.request.DeviceRegistrationRequest
import com.ninecraft.booket.core.network.request.LoginRequest
import com.ninecraft.booket.core.network.request.NotificationSettingsRequest
import com.ninecraft.booket.core.network.request.RecordRegisterRequest
Expand Down Expand Up @@ -53,8 +53,8 @@ interface ReedService {
@GET("api/v1/users/me")
suspend fun getUserProfile(): UserProfileResponse

@PUT("api/v1/users/me/fcm-token")
suspend fun updateFcmToken(@Body fcmTokenRequest: FcmTokenRequest): UserProfileResponse
@PUT("api/v1/users/me/devices")
suspend fun upsertDevice(@Body deviceRegistrationRequest: DeviceRegistrationRequest): UserProfileResponse

@PUT("api/v1/users/me/notification-settings")
suspend fun updateNotificationSettings(@Body notificationSettingsRequest: NotificationSettingsRequest): UserProfileResponse
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.ninecraft.booket.core.common.constants.WebViewConstants
import com.ninecraft.booket.core.common.utils.handleException
import com.ninecraft.booket.core.data.api.repository.AuthRepository
import com.ninecraft.booket.core.data.api.repository.RemoteConfigRepository
import com.ninecraft.booket.core.data.api.repository.UserRepository
import com.ninecraft.booket.core.model.UserState
import com.ninecraft.booket.feature.screens.LoginScreen
import com.ninecraft.booket.feature.screens.NotificationScreen
Expand All @@ -34,6 +35,7 @@ import kotlinx.coroutines.launch
class SettingsPresenter @AssistedInject constructor(
@Assisted val navigator: Navigator,
private val authRepository: AuthRepository,
private val userRepository: UserRepository,
private val remoteConfigRepository: RemoteConfigRepository,
private val analyticsHelper: AnalyticsHelper,
) : Presenter<SettingsUiState> {
Expand Down Expand Up @@ -62,6 +64,7 @@ class SettingsPresenter @AssistedInject constructor(
isLoading = true
authRepository.logout()
.onSuccess {
userRepository.resetNotificationData()
analyticsHelper.logEvent(SETTINGS_LOGOUT_COMPLETE)
navigator.resetRoot(LoginScreen())
}
Expand Down Expand Up @@ -91,6 +94,7 @@ class SettingsPresenter @AssistedInject constructor(
isLoading = true
authRepository.withdraw()
.onSuccess {
userRepository.resetNotificationData()
analyticsHelper.logEvent(SETTINGS_WITHDRAWAL_COMPLETE)
navigator.resetRoot(LoginScreen())
}
Expand Down
Loading