feat: 멀티디바이스 푸시 알림 지원을 위한 API 스펙 변경 반영#221
Conversation
- FID(Firebase Installation ID)를 기기 ID로 사용하여 FCM 토큰과 함께 서버에 등록하도록 `upsertDevice` API를 호출하는 로직 추가
WalkthroughFirebase Installations를 주입해 장치 ID를 획득하고, FCM 토큰 흐름을 장치 등록/업서트(DeviceRegistrationRequest → upsertDevice)로 전환했으며 관련 인터페이스·데이터 모델·저장소 구현과 설정 화면 호출을 업데이트합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant App as 앱
participant Repo as DefaultUserRepository
participant Firebase as FirebaseInstallations
participant Service as ReedService
participant Backend as 백엔드
rect rgb(240,255,240)
note over App,Repo: 신규 흐름 — 장치 등록/업서트
App->>Repo: syncFcmToken() / update flow
Repo->>Firebase: id() (getDeviceId)
Firebase-->>Repo: deviceId
Repo->>Service: upsertDevice(DeviceRegistrationRequest{deviceId, fcmToken})
Service->>Backend: POST /api/v1/users/me/devices
Backend-->>Service: UserProfileResponse
Service-->>Repo: 응답
Repo-->>App: 완료
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
93-105: 디바이스 등록 플로우가 올바르게 구현되었습니다.Firebase Installation ID를 사용한 디바이스 등록 로직이 정확하며, 에러 처리도 적절합니다.
runSuspendCatching으로 감싸진 호출 체인에서 예외가 올바르게 전파됩니다.선택 사항: 디바이스 등록 성공 로깅 추가
관찰성(observability) 향상을 위해
registerDevice메서드에 성공 로그를 추가하는 것을 고려해보세요:private suspend fun registerDevice(fcmToken: String) { val deviceId = getDeviceId() service.upsertDevice(DeviceRegistrationRequest(deviceId, fcmToken)) + Logger.d("Device registered successfully - deviceId: ${deviceId.take(8)}...") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt(4 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/request/DeviceRegistrationRequest.kt(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seoyoon513
Repo: YAPP-Github/Reed-Android PR: 46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
🧬 Code graph analysis (1)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (3)
core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt (1)
setFcmToken(7-7)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (1)
setFcmToken(24-28)core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt (1)
runSuspendCatching(16-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Compose Stability Check
- GitHub Check: ci-build
🔇 Additional comments (4)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/di/FirebaseModule.kt (1)
38-40: Firebase Installations 프로바이더 추가가 적절합니다.멀티디바이스 지원을 위한 Firebase Installation ID 획득에 필요한 의존성이 올바르게 제공되고 있습니다. 기존 Firebase 프로바이더 패턴과 일관성 있게 구현되었습니다.
core/network/src/main/kotlin/com/ninecraft/booket/core/network/request/DeviceRegistrationRequest.kt (1)
7-11: 디바이스 등록 요청 모델이 올바르게 업데이트되었습니다.클래스명 변경(
FcmTokenRequest→DeviceRegistrationRequest)과deviceId필드 추가가 멀티디바이스 푸시 알림 지원을 위한 API 스펙 변경을 정확히 반영하고 있습니다.core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (1)
56-57: API 엔드포인트가 새로운 스펙에 맞게 올바르게 변경되었습니다.메서드명과 엔드포인트가 디바이스 기반 등록/업데이트 방식으로 적절히 변경되었으며,
upsert네이밍이 생성 또는 업데이트 의미를 명확히 전달합니다.core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
41-57: 토큰 동기화 플로우가 새로운 디바이스 등록 방식으로 올바르게 업데이트되었습니다.두 가지
syncFcmToken메서드 모두registerDevice를 호출하도록 변경되어 멀티디바이스 지원이 정상적으로 적용되었습니다.로그아웃 시 FCM 토큰 삭제 처리 확인
PR 설명에서 언급하신 것처럼, 서버가 세션 만료 시 FCM 토큰을 별도 처리하지 않으므로 클라이언트에서 로그아웃 시 토큰 삭제가 필요합니다. 현재 코드베이스에서 로그아웃/탈퇴 구현 위치를 확인하여 어느 접근 방식이 가장 적절한지 검토하시는 것을 권장합니다.
제안하신 3가지 접근 방식 중:
- AuthRepository 내부 처리: 구현은 간단하지만 책임 분리 측면에서 고려 필요
- Presenter에서 처리: 책임 분리는 되지만 Presenter의 조합 책임 증가
- UseCase 도입: 가장 명확한 책임 분리이나 도메인 레이어 구축 비용 발생
현재 로그아웃/탈퇴 구현 위치를 파악하는 데 도움이 필요하시면 코드베이스를 검색해드릴 수 있습니다.
|
@coderabbitai 위에서 제시한 3가지 방법 중 너의 의견은? (현재 구조를 고려해서 고민해줘) |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2024 🏁 Script executed: Length of output: 17799 🏁 Script executed: Length of output: 868 @seoyoon513 현재 프로젝트 구조를 분석한 결과, 방법 2 (Presenter에서 처리) 를 추천합니다. 근거현재 프로젝트 구조 분석
각 방법에 대한 평가방법 1: AuthRepository 내부 처리
방법 2: Presenter에서 처리 ✨
방법 4: UseCase 도입
구현 예시 (방법 2)// UserRepository에 메서드 추가
interface UserRepository {
suspend fun deleteFcmToken(): Result<Unit>
}
// DefaultUserRepository 구현
override suspend fun deleteFcmToken() = runSuspendCatching {
firebaseMessaging.deleteToken().await()
notificationDataSource.setFcmToken("")
}
// SettingsPresenter에서 호출
fun logout() {
scope.launch {
try {
isLoading = true
userRepository.deleteFcmToken() // FCM 토큰 먼저 삭제
authRepository.logout()
.onSuccess {
analyticsHelper.logEvent(SETTINGS_LOGOUT_COMPLETE)
navigator.resetRoot(LoginScreen())
}
// ... error handling
} finally {
isLoading = false
}
}
}추후 고려사항만약 향후 다른 곳(예: 토큰 만료 시 자동 로그아웃)에서도
현재로서는 방법 2가 가장 현실적이고 구조적으로 적절합니다. 🧠 Learnings used |
… 알림 관련 데이터 초기화 진행
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (2)
61-89: 로그아웃 플로우에서 불일치 상태 발생 가능성현재 구현에서
authRepository.logout()이 성공한 후resetNotificationData()가 실패하면, 사용자는 서버에서는 로그아웃되었지만 로그인 화면으로 이동하지 않고 설정 화면에 남게 됩니다. 이는 불일치 상태를 만들 수 있습니다.다음 중 하나의 해결 방법을 권장합니다:
방법 1 (권장): Best-effort 토큰 정리
authRepository.logout() .onSuccess { - userRepository.resetNotificationData() + runCatching { userRepository.resetNotificationData() } + .onFailure { Logger.e("Failed to reset notification data: ${it.message}") } analyticsHelper.logEvent(SETTINGS_LOGOUT_COMPLETE) navigator.resetRoot(LoginScreen()) }방법 2: 별도 try-catch로 처리
authRepository.logout() .onSuccess { + try { + userRepository.resetNotificationData() + } catch (e: Exception) { + Logger.e("Failed to reset notification data: ${e.message}") + } analyticsHelper.logEvent(SETTINGS_LOGOUT_COMPLETE) navigator.resetRoot(LoginScreen()) }PR 설명에 따르면 서버는 로그아웃 시 FCM 토큰을 별도 처리하지 않으므로, 토큰 정리 실패가 치명적이지는 않습니다. 하지만 사용자는 서버 로그아웃 성공 시 항상 로그인 화면으로 이동해야 합니다.
91-119: 탈퇴 플로우에도 동일한 이슈 적용
logout()메서드와 동일한 불일치 상태 문제가withdraw()메서드에도 존재합니다. 위의 로그아웃 플로우 리뷰 코멘트에서 제안한 동일한 수정을 적용해주세요.authRepository.withdraw() .onSuccess { - userRepository.resetNotificationData() + runCatching { userRepository.resetNotificationData() } + .onFailure { Logger.e("Failed to reset notification data: ${it.message}") } analyticsHelper.logEvent(SETTINGS_WITHDRAWAL_COMPLETE) navigator.resetRoot(LoginScreen()) }
🧹 Nitpick comments (2)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/UserRepository.kt (1)
31-31: 인터페이스 추가 확인 완료메서드 시그니처와 위치가 적절합니다. 알림 관련 메서드들과 함께 그룹화되어 있어 가독성이 좋습니다.
향후 유지보수성을 위해 KDoc 추가를 고려해보세요:
+ /** + * 알림 관련 데이터를 초기화합니다. + * 원격 FCM 토큰 삭제 및 로컬 알림 데이터 스토어를 클리어합니다. + */ suspend fun resetNotificationData()core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
74-77: 알림 데이터 초기화 구현 검토현재 구현은 적절하지만, 예외 처리 전략을 명확히 하는 것이 좋습니다.
deleteRemoteFcmToken()은 실패를 로그만 남기고 무시하지만,clearNotificationDataStore()는 예외를 전파할 수 있습니다. 일관성을 위해 다음과 같이 수정하는 것을 권장합니다:override suspend fun resetNotificationData() { deleteRemoteFcmToken() - clearNotificationDataStore() + try { + clearNotificationDataStore() + } catch (e: Exception) { + Logger.e("Failed to clear notification data store: ${e.message}") + } }이렇게 하면
resetNotificationData()가 항상 성공하며(never throws), 상위 호출자(SettingsPresenter)에서 예외 처리를 걱정할 필요가 없습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/UserRepository.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt(0 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt(5 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt(4 hunks)
💤 Files with no reviewable changes (1)
- core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: seoyoon513
Repo: YAPP-Github/Reed-Android PR: 46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
📚 Learning: 2025-08-28T12:25:54.058Z
Learnt from: easyhooon
Repo: YAPP-Github/Reed-Android PR: 174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.058Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
Applied to files:
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt
🧬 Code graph analysis (1)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (3)
core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt (2)
setFcmToken(7-7)clearNotificationDataStore(15-15)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (2)
setFcmToken(24-28)clearNotificationDataStore(54-58)core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt (1)
runSuspendCatching(16-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci-build
- GitHub Check: Compose Stability Check
🔇 Additional comments (7)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (1)
13-13: 의존성 주입 추가 확인 완료UserRepository 의존성이 기존 패턴과 일관되게 추가되었습니다.
Also applies to: 38-38
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (6)
3-3: 의존성 추가 확인 완료FirebaseInstallations 의존성과 DeviceRegistrationRequest 사용이 적절하게 추가되었습니다.
Also applies to: 10-10, 25-25
41-57: FCM 토큰 동기화 로직 업데이트 확인 완료
updateFcmToken에서registerDevice로의 마이그레이션이 깔끔하게 처리되었습니다.runSuspendCatching으로 예외 처리가 적절하게 되어 있습니다.
98-105: 디바이스 ID 조회 로직 확인 완료Firebase Installations ID를 가져오는 로직이 적절하게 구현되었습니다. 에러 로깅 후 재throw하는 패턴이
getRemoteFcmToken()과 일관성 있게 적용되었습니다.
107-110: 디바이스 등록 로직 확인 완료
DeviceRegistrationRequest를 사용한 디바이스 등록 구현이 깔끔합니다. 호출자의runSuspendCatching에서 예외 처리가 되므로 적절합니다.
112-118: 원격 FCM 토큰 삭제 로직 확인 완료Best-effort 방식으로 원격 토큰 삭제를 시도하되, 실패해도 로컬 데이터 정리를 계속 진행하는 전략이 적절합니다. PR 설명에서 언급된 "서버는 로그아웃 시 FCM 토큰을 별도 처리하지 않음"과 일치하는 구현입니다.
120-122: 로컬 알림 데이터 클리어 로직 확인 완료DataSource로의 단순 위임이 적절합니다. 개선 제안은
resetNotificationData()리뷰 코멘트를 참고하세요.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
111-114: 디바이스 등록 로직 구현 확인Device ID 획득 후 서버에 디바이스를 등록하는 로직이 올바르게 구현되었습니다. 에러 처리는 호출자(
syncFcmToken)의runSuspendCatching에서 처리되므로 현재 구조가 적절합니다.디버깅 편의를 위해 서비스 호출 실패 시 로깅을 추가하는 것을 고려할 수 있습니다:
private suspend fun registerDevice(fcmToken: String) { val deviceId = getDeviceId() try { service.upsertDevice(DeviceRegistrationRequest(deviceId, fcmToken)) } catch (e: Exception) { Logger.e("Failed to register device: ${e.message}") throw e } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/UserRepository.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seoyoon513
Repo: YAPP-Github/Reed-Android PR: 46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
🧬 Code graph analysis (1)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (3)
core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt (2)
setFcmToken(7-7)clearNotificationDataStore(15-15)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (2)
setFcmToken(24-28)clearNotificationDataStore(54-58)core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt (1)
runSuspendCatching(16-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Compose Stability Check
- GitHub Check: ci-build
🔇 Additional comments (5)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (4)
3-3: 의존성 추가 확인 완료Firebase Installations 의존성이 올바르게 추가되었으며, 생성자 주입 패턴을 정확히 따르고 있습니다.
DeviceRegistrationRequest로의 이름 변경도 멀티디바이스 지원 목적에 부합합니다.Also applies to: 10-10, 25-25
41-57: FCM 토큰 동기화 로직 업데이트 확인
updateFcmToken에서registerDevice로의 변경이 올바르게 적용되었습니다. 기존의runSuspendCatching래퍼를 통한 에러 처리도 유지되어 안정적입니다.
102-109: Device ID 획득 로직 구현 확인Firebase Installation ID(FID)를 올바르게 획득하고 있으며, 에러 처리도 기존 패턴(
getRemoteFcmToken)과 일관되게 구현되었습니다. 멀티디바이스 지원의 핵심 변경사항이 정확히 반영되었습니다.
116-122: 정리 헬퍼 메서드 구현 확인FCM 토큰 삭제 및 로컬 데이터 정리를 위한 헬퍼 메서드들이 명확하게 분리되어 구현되었습니다. 단일 책임 원칙을 잘 따르고 있으며, 에러 처리는 호출자에서 수행되는 구조가 적절합니다.
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/UserRepository.kt (1)
33-33: 검증 결과: 의도적인 설계로 보이지만 일관성 문제 유지원본 리뷰 의견의 우려는 부분적으로 타당합니다. 다음 사항이 확인되었습니다:
일관성 부족 -
syncFcmToken(),updateNotificationSettings()등 다른 저장소 메서드들은Result<Unit>또는Result<T>를 반환하지만,resetNotificationData()는 반환값이 없습니다.의도적인 설계 - 구현부에서 모든 예외를 catch하여 로그만 기록하고 예외를 전파하지 않습니다. 이는 "최선형 노력" 정리 작업 패턴입니다.
호출 방식 -
SettingsPresenter에서 로그아웃/탈퇴 성공 후 호출되며, 반환값이 무시되고 실패해도 메인 플로우를 차단하지 않습니다.관찰사항: 현재 구조는 작동하지만, 다음 중 하나를 고려하세요:
- 정리 작업 실패를 사용자에게 보고해야 한다면
Result<Unit>반환 추가- 현재 "최선형 노력" 방식이 의도라면, 그 의도를 코드 주석으로 명시
| override suspend fun resetNotificationData() { | ||
| try { | ||
| deleteRemoteFcmToken() | ||
| clearNotificationDataStore() | ||
| } catch (e: Exception) { | ||
| Logger.e("Failed to reset notification data: ${e.message}") | ||
| } | ||
| } |
There was a problem hiding this comment.
에러 처리 일관성 및 부분 실패 처리 검토 필요
다음 사항들을 확인해 주세요:
-
에러 처리 불일치: 이 메서드는 예외를 로깅만 하고 삼키는 반면,
getRemoteFcmToken(),getDeviceId()등 다른 메서드들은 예외를 다시 던집니다. -
부분 실패 케이스:
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).
|
저는 앱 재설치시에도 그 값이 변하지않은 SSAID를 Devide 고유 식별 ID로 주로 사용했었는데, FID도 생각해보니 괜찮은것 같군요(앱 재설치시지 fcmToken 역시 새로 발급되므로 1:1 매핑 구조, 자동으로 다른 디바이스로 인식) 다만, 서버에서 한 사용자에 대한 등록 기기가 계속 증가하게 되는 구조이므로(앱 재설치마다, FID 변경 -> 다른 기기라고 판단) |
@seoyoon513 2,3 번 방식 모두 괜찮다고 생각합니다. 그런데 현재 refreshToken 까지 만료된 경우, logout API를 호출하고 있진 않군요. 서버에서 accessToken, refreshToken에 대한 블랙리스트 처리를 하고 있다면, 서버 정책에 따라 필요할것으로도 보여지는데 확인을 해봐야겠네요 만약에 logout API를 호출해야한다면 그에 따라 모든 presenter에서 반복적으로 호출될 필요가 있어, UseCase로 따로 빼는게 더 나을듯 합니다. |
그런데 logout api 자체가 auth required인데, refreshToken까지 만료된 경우면 401에러가 발생하지 않을까요? |
그르네여 그땐 logout API 호출이 의미가 없군여. 지금대로 하면 될 것 같슴다 |
서버에서 어떤 방식으로 upsert를 진행하는지에 따라 달라지겠군요. 해당 부분은 서버에 노티가 필요하겠네요 |
easyhooon
left a comment
easyhooon
left a comment
There was a problem hiding this comment.
iOS에서는 DeviceId를 어떤 것을 사용하는지 확인해보고, FID처럼 재설치시 갱신되는 것을 사용한다면 위에 코멘트에 언급했던 서버 로직 요청 해두면 될 것 같슴다. 수고하셨습니다~
정확힌 upsert 라기 보단 사후적인 delete 라고 해야하나 |
2 participants
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역
💬 추가 설명 or 리뷰 포인트
지훈님 의견이 궁급함니다
1. AuthRepository 내부에서 처리
2. Presenter에서
logout(),withdraw()호출 시점에서 처리3. UseCase 도입
=> 일단 토끼가 제안한 2번 방식으로 진행하려고 합니다
Summary by CodeRabbit
새로운 기능
버그 수정 / 개선