[FEAT] FCM Token 발급, 등록, 푸시알림 기능 구현 및 기타 QA수정#136
[FEAT] FCM Token 발급, 등록, 푸시알림 기능 구현 및 기타 QA수정#136Nico1eKim merged 33 commits intoTHIP-TextHip:developfrom
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughFirebase Cloud Messaging integration was added: dependencies, manifest, messaging service, token management, repository/service APIs, device ID utils, runtime permission handling, and UI to view/toggle notification enable state. Additional edits include feed image upload concurrency, account deletion cleanup with FCM token removal, minor UI/resource updates, and formatting refactors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as App (Activity/VM)
participant FCM as FirebaseMessaging
participant FTM as FcmTokenManager
participant NR as NotificationRepository
participant NS as NotificationService
participant API as Server API
rect rgb(230,245,255)
note over App: App start / login / signup
User->>App: Open app / Authenticate
App->>FTM: sendCurrentTokenIfExists()
alt Token stored locally
FTM->>NR: registerFcmToken(deviceId, token)
else No local token
FTM->>FCM: getInstance().token
FCM-->>FTM: token
FTM->>NR: registerFcmToken(deviceId, token)
end
NR->>NS: registerFcmToken(request)
NS->>API: POST /notifications/fcm-tokens
API-->>NS: BaseResponse
NS-->>NR: Response
NR-->>FTM: Result
end
sequenceDiagram
autonumber
participant FMS as MyFirebaseMessagingService
participant FTM as FcmTokenManager
participant NR as NotificationRepository
participant NS as NotificationService
participant API as Server API
rect rgb(240,255,240)
note over FMS: FCM token refresh
FMS-->>FMS: onNewToken(token)
FMS->>FTM: handleNewToken(token)
FTM->>NR: registerFcmToken(deviceId, token)
NR->>NS: registerFcmToken(request)
NS->>API: POST /notifications/fcm-tokens
API-->>NS: BaseResponse
NS-->>NR: Response
NR-->>FTM: Result
end
sequenceDiagram
autonumber
actor User
participant Screen as MyPageNotificationEditScreen
participant VM as MypageNotificationEditViewModel
participant Repo as NotificationRepository
participant Svc as NotificationService
participant API as Server API
rect rgb(255,245,230)
note over VM: 초기 로딩
Screen->>VM: collect uiState
VM->>Repo: getNotificationEnableState()
Repo->>Svc: GET /users/notification-settings?deviceId=...
Svc->>API: Request
API-->>Svc: Response (isEnabled)
Svc-->>Repo: BaseResponse
Repo-->>VM: Result<NotificationEnabledResponse?>
VM-->>Screen: uiState(isEnabled)
end
rect rgb(255,240,240)
note over Screen,VM: 토글 변경
User->>Screen: Toggle on/off
Screen->>VM: onNotificationToggle(enabled)
VM->>Repo: updateNotificationEnabled(enabled)
Repo->>Svc: PATCH /notifications/enable-state
Svc->>API: Request
API-->>Svc: Response
Svc-->>Repo: BaseResponse
Repo-->>VM: Result
VM-->>Screen: uiState(updated or rollback)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewersPoem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR implements Firebase Cloud Messaging (FCM) functionality for push notifications, including token generation, registration, and notification management features. It also includes various QA fixes and updates throughout the app.
- FCM token generation and registration with server
- Push notification enable/disable settings management
- Notification screen UI updates and improvements
- General QA fixes and code formatting improvements
Reviewed Changes
Copilot reviewed 31 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Added Firebase messaging dependency |
| app/src/main/res/values/strings.xml | Updated leave account notice text content |
| app/src/main/java/com/texthip/thip/utils/auth/DeviceUtils.kt | Added device ID utility function |
| app/src/main/java/com/texthip/thip/service/MyFirebaseMessagingService.kt | Implemented FCM message handling service |
| app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt | Added FCM token management functionality |
| app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt | Implemented notification API repository |
| app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/MypageNotificationEditViewModel.kt | Added notification settings view model |
| app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt | Updated notification settings screen |
| Various ViewModels | Integrated FCM token sending on login/signup |
| app/src/main/AndroidManifest.xml | Registered FCM service |
| app/build.gradle.kts | Added Firebase messaging dependency |
Files not reviewed (1)
- .idea/appInsightsSettings.xml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import com.texthip.thip.data.manager.TokenManager | ||
| import dagger.hilt.android.lifecycle.HiltViewModel | ||
| import jakarta.inject.Inject | ||
| import javax.inject.Inject |
There was a problem hiding this comment.
Import statement changed from jakarta.inject.Inject to javax.inject.Inject. Ensure this change is consistent across the entire codebase to avoid dependency conflicts.
| private val APP_TOKEN_KEY = stringPreferencesKey("app_token") // 정식 액세스토큰 | ||
| private val TEMP_TOKEN_KEY = stringPreferencesKey("temp_token") // 임시 토큰 | ||
| private val REFRESH_TOKEN_KEY = stringPreferencesKey("refresh_token") | ||
| //private val REFRESH_TOKEN_KEY = stringPreferencesKey("refresh_token") |
There was a problem hiding this comment.
Instead of commenting out the refresh token functionality, consider removing it entirely if it's not being used, or add a TODO comment explaining when it will be implemented.
| /** | ||
| * 도서 장르를 나타내는 enum class | ||
| */ | ||
|
|
There was a problem hiding this comment.
[nitpick] The multi-line comment explaining the enum class was removed. Consider keeping documentation for public APIs and data models to maintain code clarity.
| /** | |
| * Represents the genre category for content in the application. | |
| * | |
| * @property displayKey The key used for display and identification. | |
| * @property apiCategory The category name used in the API. | |
| * @property networkApiCategory The category name used for network API calls (defaults to [apiCategory]). | |
| * | |
| * Enum values: | |
| * - LITERATURE: Literature genre. | |
| * - SCIENCE_IT: Science and IT genre. | |
| * - SOCIAL_SCIENCE: Social science genre. | |
| * - HUMANITIES: Humanities genre. | |
| * - ART: Art genre. | |
| */ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (18)
app/src/main/java/com/texthip/thip/ThipApplication.kt (1)
21-23: printStackTrace() 대신 구조화된 로깅 사용 제안운영 환경에서는
printStackTrace()보다Log.e/Timber/Crashlytics 등으로 실패 원인을 남기는 편이 낫습니다.적용 예시(안드로이드 기본 Log 사용):
- } catch (e: Exception) { - e.printStackTrace() + } catch (e: Exception) { + Log.e("ThipApplication", "Kakao SDK init failed", e)파일 상단 import 추가:
+import android.util.Logapp/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt (1)
137-137: Top padding(8dp) 추가로 시각적 중앙이 하향됩니다 — 의도 확인 요청Box 높이(42dp)에서 top padding 8dp를 주면 내부 영역이 34dp가 되어 CenterStart 정렬 텍스트가 전체 바 기준 약 4dp 아래로 내려가 보입니다. 디자인 의도인지 확인 부탁드립니다.
app/src/main/java/com/texthip/thip/data/manager/Genre.kt (1)
3-3: 공개 enum KDoc 제거 — 유지 권장외부/다른 모듈에서 참조되는 공개 타입이므로 KDoc을 보존하면 IDE 툴팁/문서화에 도움이 됩니다.
app/src/main/java/com/texthip/thip/data/model/notification/response/NotificationEnabledResponse.kt (1)
5-8: 서버 필드 누락 시 복원력 확보를 위한 기본값 제안스키마 변경/부분 응답 대비 기본값을 두면 역직렬화 안전성이 높아집니다.
제안 diff:
-@Serializable -data class NotificationEnabledResponse( - val isEnabled: Boolean -) +@Serializable +data class NotificationEnabledResponse( + val isEnabled: Boolean = false +)gradle/libs.versions.toml (1)
30-30: Firebase BoM로 전환 — firebase-messaging 개별 버전 제거 및 BoM(34.2.0) 적용firebase-messaging을 개별 버전(현재 25.0.0)으로 고정하지 말고 Firebase BoM으로 관리하세요. 최신 안정 BoM: 34.2.0 (검증: 2025-09-14).
제안 diff (gradle/libs.versions.toml):
+[versions] +# ...기존 유지... -firebaseMessaging = "25.0.0" +firebaseBom = "34.2.0" [libraries] # ...기존 유지... -firebase-messaging = { group = "com.google.firebase", name = "firebase-messaging", version.ref = "firebaseMessaging" } +firebase-bom = { module = "com.google.firebase:firebase-bom", version.ref = "firebaseBom" } +firebase-messaging = { module = "com.google.firebase:firebase-messaging" }app/build.gradle.kts에:
implementation(platform(libs.firebase.bom)) implementation(libs.firebase.messaging)app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (2)
92-99: 메타데이터 수집 중 단일 예외로 전체 취소 위험
getImageMetadata내부에서 예외가 나면 전체 코루틴이 취소됩니다. 개별 실패만 건너뛰도록 캡슐화하는 편이 안전합니다.아래처럼
runCatching으로 감싸면 부분 실패 허용이 가능합니다.- val validImagePairs = imageUris.map { uri -> - async { - imageUploadHelper.getImageMetadata(uri)?.let { metadata -> - uri to metadata - } - } - }.awaitAll().filterNotNull() + val validImagePairs = imageUris.map { uri -> + async { + runCatching { imageUploadHelper.getImageMetadata(uri) } + .getOrNull() + ?.let { metadata -> uri to metadata } + } + }.awaitAll().filterNotNull()
104-110: 예외 타입을 구체화하여 오류 원인 전달력 개선일반
Exception은 원인 분류가 어렵습니다. 상태 불일치는IllegalStateException, 업로드 실패는IOException등 구체 타입을 사용하세요.- .getOrThrow() ?: throw Exception("Failed to get presigned URLs") + .getOrThrow() ?: throw IllegalStateException("Failed to get presigned URLs") - if (validImagePairs.size != presignedResponse.presignedUrls.size) { - throw Exception("개수가 올바르지 않습니다: expected ${validImagePairs.size}, got ${presignedResponse.presignedUrls.size}") - } + if (validImagePairs.size != presignedResponse.presignedUrls.size) { + throw IllegalStateException("개수가 올바르지 않습니다: expected ${validImagePairs.size}, got ${presignedResponse.presignedUrls.size}") + }- }.onFailure { exception -> - throw Exception("Failed to upload image ${index + 1}: ${exception.message}") - } + }.onFailure { exception -> + throw java.io.IOException("Failed to upload image ${index + 1}", exception) + }필요 시:
import java.io.IOExceptionAlso applies to: 121-125
app/build.gradle.kts (1)
106-106: FCM 의존성: BoM 및 KTX 사용 고려Firebase는 BoM 사용과
-ktx아티팩트를 권장합니다. 다수 Firebase 라이브러리 추가 시 버전 호환성이 쉬워집니다.예시(버전은 버전 카탈로그/사내 기준에 맞춰 조정):
dependencies { implementation(platform(libs.firebase.bom)) implementation("com.google.firebase:firebase-messaging-ktx") }BoM alias가 없으면
libs.versions.toml에 추가한 뒤 사용해주세요.app/src/main/java/com/texthip/thip/ui/navigator/navigations/MyPageNavigation.kt (1)
6-12: 네이밍 일관성(Nit): MyPage vs Mypage 혼재
MyPageScreen,MypageCustomerServiceScreen등 케이스가 혼재합니다. 모듈 전반에서MyPage로 통일 권장합니다(파일명/클래스/패키지).app/src/main/java/com/texthip/thip/service/MyFirebaseMessagingService.kt (4)
52-54: 비구조화된 코루틴 스코프 사용을 개선해주세요.
CoroutineScope(Dispatchers.IO).launch는 구조화된 동시성 원칙을 위반합니다. 서비스의 생명주기와 연결된 스코프를 사용하거나lifecycleScope를 고려해주세요.- CoroutineScope(Dispatchers.IO).launch { - fcmTokenManager.handleNewToken(token) - } + // 서비스 생명주기와 연결된 스코프 사용 권장 + // 또는 GlobalScope를 명시적으로 사용 (토큰 저장은 앱 전체 수명과 관련) + GlobalScope.launch(Dispatchers.IO) { + fcmTokenManager.handleNewToken(token) + }
62-67: PendingIntent 요청 코드를 고유하게 만들어주세요.현재 요청 코드가 0으로 하드코딩되어 있어 여러 알림이 동시에 처리될 때 문제가 발생할 수 있습니다.
val pendingIntent = PendingIntent.getActivity( this, - 0, + System.currentTimeMillis().toInt(), intent, PendingIntent.FLAG_ONE_SHOT or PendingIntent.FLAG_IMMUTABLE )
80-80: 알림 ID를 고유하게 만들어주세요.현재 알림 ID가 0으로 하드코딩되어 있어 새 알림이 이전 알림을 덮어쓸 수 있습니다.
- notificationManager.notify(0, notificationBuilder.build()) + notificationManager.notify(System.currentTimeMillis().toInt(), notificationBuilder.build())
72-72: 적절한 알림 아이콘을 사용해주세요.현재
ic_launcher_foreground를 사용 중인데, 알림 전용 아이콘을 사용하는 것이 좋습니다. 전경 아이콘은 알림에 최적화되지 않았을 수 있습니다.- .setSmallIcon(R.drawable.ic_launcher_foreground) + .setSmallIcon(R.drawable.ic_notification) // 알림 전용 아이콘 사용 권장app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt (2)
68-69: 날짜 포맷 지역화 개선 필요현재 날짜 포맷이 하드코딩되어 있습니다. 앱이 다국어를 지원할 경우 문제가 될 수 있습니다.
날짜 포맷을 리소스로 관리하는 것을 고려해보세요:
-val dateFormat = SimpleDateFormat("yyyy년 M월 d일 H시 m분", Locale.KOREAN) +val dateFormat = SimpleDateFormat(stringResource(R.string.toast_date_format), Locale.getDefault())
strings.xml에 추가:<string name="toast_date_format">yyyy년 M월 d일 H시 m분</string>
53-58: Toast 지속 시간이 하드코딩됨Toast 표시 시간이 3000ms로 하드코딩되어 있습니다. 유지보수성을 위해 상수로 분리하는 것이 좋습니다.
+companion object { + private const val TOAST_DISPLAY_DURATION_MS = 3000L +} LaunchedEffect(toastMessage) { if (toastMessage != null) { - delay(3000) + delay(TOAST_DISPLAY_DURATION_MS) toastMessage = null } }app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/MypageNotificationEditViewModel.kt (1)
35-66: 동일한 로직에서 중복된 상태 업데이트 패턴
fetchNotificationEnableState()에서 성공/실패 케이스의 상태 업데이트 로직이 중복되어 있습니다.헬퍼 함수를 활용하여 중복을 제거할 수 있습니다:
+private fun handleLoadingComplete( + isEnabled: Boolean? = null, + errorMessage: String? = null +) { + updateState { + it.copy( + isLoading = false, + isNotificationEnabled = isEnabled ?: it.isNotificationEnabled, + errorMessage = errorMessage + ) + } +} fun fetchNotificationEnableState() { viewModelScope.launch { updateState { it.copy(isLoading = true) } notificationRepository.getNotificationEnableState() .onSuccess { data -> - data?.let { - updateState { - it.copy( - isLoading = false, - isNotificationEnabled = data.isEnabled, - errorMessage = null - ) - } - } ?: run { - updateState { - it.copy( - isLoading = false, - errorMessage = "알림 설정 정보를 가져올 수 없습니다." - ) - } - } + if (data != null) { + handleLoadingComplete(isEnabled = data.isEnabled) + } else { + handleLoadingComplete(errorMessage = "알림 설정 정보를 가져올 수 없습니다.") + } } .onFailure { exception -> - updateState { - it.copy( - isLoading = false, - errorMessage = exception.message - ) - } + handleLoadingComplete(errorMessage = exception.message) } } }app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt (1)
31-40: 토큰 비교 로직의 경쟁 조건 가능성
getFcmTokenOnce()와saveFcmToken()사이에 다른 토큰 업데이트가 발생할 수 있는 경쟁 조건이 있습니다.원자적 업데이트를 보장하려면 DataStore의
updateData를 사용하는 것이 좋습니다:suspend fun handleNewToken(newToken: String) { - val storedToken = getFcmTokenOnce() - - if (storedToken != newToken) { - Log.d("FCM", "Token updated") - - saveFcmToken(newToken) - sendTokenToServer(newToken) + dataStore.edit { prefs -> + val storedToken = prefs[FCM_TOKEN_KEY] + if (storedToken != newToken) { + Log.d("FCM", "Token updated") + prefs[FCM_TOKEN_KEY] = newToken + } + }.also { + // 토큰이 변경된 경우에만 서버로 전송 + val currentToken = getFcmTokenOnce() + if (currentToken == newToken) { + sendTokenToServer(newToken) + } } }app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt (1)
26-26: 플랫폼 타입이 하드코딩됨"ANDROID" 문자열이 하드코딩되어 있습니다. 상수로 정의하는 것이 좋습니다.
+companion object { + private const val PLATFORM_TYPE_ANDROID = "ANDROID" +} val request = FcmTokenRequest( deviceId = deviceId, fcmToken = fcmToken, - platformType = "ANDROID" + platformType = PLATFORM_TYPE_ANDROID )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (33)
.idea/appInsightsSettings.xml(2 hunks)app/build.gradle.kts(1 hunks)app/src/main/AndroidManifest.xml(1 hunks)app/src/main/java/com/texthip/thip/MainActivity.kt(2 hunks)app/src/main/java/com/texthip/thip/MainScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ThipApplication.kt(2 hunks)app/src/main/java/com/texthip/thip/data/di/DataStoreModule.kt(1 hunks)app/src/main/java/com/texthip/thip/data/di/ServiceModule.kt(2 hunks)app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt(1 hunks)app/src/main/java/com/texthip/thip/data/manager/Genre.kt(1 hunks)app/src/main/java/com/texthip/thip/data/manager/TokenManager.kt(2 hunks)app/src/main/java/com/texthip/thip/data/model/notification/request/FcmTokenRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/notification/request/NotificationEnabledRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/notification/response/NotificationEnabledResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt(3 hunks)app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt(4 hunks)app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/RecentSearchRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/RoomsRepository.kt(5 hunks)app/src/main/java/com/texthip/thip/data/repository/UserRepository.kt(3 hunks)app/src/main/java/com/texthip/thip/data/service/NotificationService.kt(1 hunks)app/src/main/java/com/texthip/thip/service/MyFirebaseMessagingService.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageLeavethipScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/MypageNotificationEditViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/MyPageNavigation.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/signin/viewmodel/LoginViewModel.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/signin/viewmodel/SignupViewModel.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/signin/viewmodel/SplashViewModel.kt(3 hunks)app/src/main/java/com/texthip/thip/utils/auth/DeviceUtils.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)gradle/libs.versions.toml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/src/main/java/com/texthip/thip/ui/navigator/navigations/MyPageNavigation.kt (1)
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt (1)
MyPageNotificationEditScreen(44-72)
app/src/main/java/com/texthip/thip/ui/signin/viewmodel/SignupViewModel.kt (1)
app/src/main/java/com/texthip/thip/ui/signin/viewmodel/SplashViewModel.kt (1)
sendFcmToken(49-53)
app/src/main/java/com/texthip/thip/ui/signin/viewmodel/SplashViewModel.kt (1)
app/src/main/java/com/texthip/thip/ui/signin/viewmodel/SignupViewModel.kt (1)
sendFcmToken(160-164)
🔇 Additional comments (32)
app/src/main/java/com/texthip/thip/ThipApplication.kt (1)
11-11: 포맷팅 변경 — LGTM기능 변화 없음. 그대로 진행해도 됩니다.
app/src/main/java/com/texthip/thip/data/repository/RecentSearchRepository.kt (2)
15-21: 매직 문자열(type) → 타입 세이프화 + null 계약 확인 필요File: app/src/main/java/com/texthip/thip/data/repository/RecentSearchRepository.kt
- 파라미터
type: String은 오타/허용치 밖 값 유입 위험 — enum/Sealed class로 좁힐 것.- type: String + type: SearchType
- 반환 타입
Result<RecentSearchResponse?>는 널 허용 — 서버가 실제로 널 바디(예: 204 No Content)를 반환하는지 확인. 불가능하면requireNotNull(...getOrThrow())로 널 금지.- 호출부 영향도 확인 필요 — 자동 탐색 시도 중
rg --type=kt에러 발생(파일 타입 인식 실패). 로컬에서 아래 명령으로 호출부 확인:rg -nP -C1 '\bgetRecentSearches\s*\(' -g '**/*.kt' rg -nP -C2 '\bgetRecentSearches\s*\([^)]*"[^"]+' -g '**/*.kt'
- KDoc에
@param type설명 추가 권장.
24-31: 삭제 API: getOrThrow() 호출 뒤 명시적 Unit 반환 적용 권장handleBaseResponse()는 실패 시 Result.failure(...)을 반환해 .getOrThrow()가 예외를 던지므로, 성공 케이스에서 바디가 null이어도 getOrThrow() 호출 뒤 명시적으로 Unit을 반환하면 의미상 동일하고 가독성이 더 좋습니다.
): Result<Unit> = runCatching { recentSearchService.deleteRecentSearch(recentSearchId) .handleBaseResponse() .getOrThrow() - ?: Unit + Unit }app/src/main/java/com/texthip/thip/MainScreen.kt (1)
41-41: 형식 변경만: 문제 없음빈 줄 추가만 있으며 동작에 영향 없습니다.
app/src/main/java/com/texthip/thip/MainActivity.kt (1)
25-25: 형식 변경만: 문제 없음주입/내비게이션 흐름에는 영향 없습니다.
Also applies to: 52-52
app/src/main/java/com/texthip/thip/data/model/notification/request/FcmTokenRequest.kt (1)
5-10: FCM 토큰 등록 요청 모델 구현 완료새로운 FCM 토큰 등록을 위한 데이터 모델이 적절히 구현되었습니다. platformType의 기본값 "ANDROID"도 안드로이드 앱에 적합합니다.
app/src/main/java/com/texthip/thip/data/model/notification/request/NotificationEnabledRequest.kt (1)
5-9: 알림 활성화 요청 모델 구현 완료알림 설정 변경을 위한 요청 모델이 깔끔하게 구현되었습니다. Boolean 타입의 enable과 String 타입의 deviceId로 구성이 적절합니다.
app/src/main/java/com/texthip/thip/data/manager/TokenManager.kt (2)
21-21: Refresh 토큰 키 주석 처리Refresh 토큰 관련 상수가 주석 처리되었습니다. 향후 필요시 활성화할 수 있도록 유지되어 있는 것이 좋습니다.
57-63: Refresh 토큰 관련 메서드 주석 처리Refresh 토큰 저장 및 조회 메서드가 블록 주석으로 처리되어 있습니다. 현재는 사용하지 않지만 확장성을 고려한 적절한 처리입니다.
.idea/appInsightsSettings.xml (2)
4-4: Firebase Crashlytics 탭 선택App Insights에서 Firebase Crashlytics 탭이 기본 선택되도록 설정되었습니다.
27-29: Firebase 프로젝트 설정 업데이트Firebase Crashlytics 연결 정보가 새로운 프로젝트 설정으로 업데이트되었습니다:
- mobileSdkAppId, projectId, projectNumber 모두 새로운 값으로 변경
- FCM 기능 추가와 관련된 Firebase 프로젝트 설정 변경으로 추정됩니다.
app/src/main/res/values/strings.xml (3)
114-121: 회원탈퇴 안내 문구 세분화회원탈퇴 안내 문구가 더 세밀하게 분리되어 UI에서 강조 표시를 위한 구조로 개선되었습니다:
- "즉시 삭제"를 별도 리소스(leave_thip_notice_1_2)로 분리하여 빨간색 강조 가능
- "되며,"를 별도 리소스(leave_thip_notice_1_3)로 분리하여 자연스러운 문장 연결
117-120: 회원탈퇴 안내 내용 개선안내 문구가 보다 구체적이고 명확하게 개선되었습니다:
- "복구 불가능" → "복구가 불가능" (문법 개선)
- 백업 및 로그 삭제, 법령상 보존 의무 정보에 대한 구체적 안내 추가
121-121: 동의 문구 개선"확인하였으며" → "이해하였으며"로 변경하여 사용자가 내용을 충분히 숙지했음을 강조하는 표현으로 개선되었습니다.
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageLeavethipScreen.kt (2)
107-110: "즉시 삭제" 강조 표시 구현새로 추가된 leave_thip_notice_1_2 ("즉시 삭제") 문자열에 빨간색 스타일을 적용하여 중요한 정보를 시각적으로 강조하도록 구현되었습니다.
116-116: Typography 스타일 통일회원탈퇴 안내 텍스트의 Typography가
feedcopy_r400_s14_h20로 통일되어 일관된 UI를 제공하도록 개선되었습니다.Also applies to: 122-122, 128-128
app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (2)
22-30: 파라미터 목록 다중라인 포맷팅getBooks 함수의 파라미터 목록이 다중라인 형태로 포맷팅되어 가독성이 향상되었습니다. 함수 동작과 반환 타입은 변경되지 않았습니다.
44-49: 일관된 코드 스타일 적용여러 함수들(getMostSearchedBooks, getBookDetail, saveBook, getSavedBooks)의 파라미터 목록이 다중라인 형태로 일관되게 포맷팅되어 코드 스타일이 통일되었습니다.
Also applies to: 52-58, 61-68, 80-86
app/src/main/java/com/texthip/thip/data/repository/RoomsRepository.kt (1)
47-50: 코드 스타일 일관성 개선여러 함수들의 파라미터 목록이 다중라인 형태로 포맷팅되어 다른 Repository 클래스들과 일관된 코드 스타일을 유지하도록 개선되었습니다. 함수 로직이나 반환 타입에는 변경이 없습니다.
Also applies to: 61-63, 83-86, 92-95, 103-107, 130-133
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (4)
91-91: IO 디스패처로 오프로딩 OK네트워크/파일 IO를
withContext(Dispatchers.IO)로 오프로딩한 선택 적절합니다.
140-143: 시그니처 포매팅 변경만 — OK동작 변경 없이 가독성만 개선되었습니다.
167-170: 시그니처 포매팅 변경만 — OK문제 없습니다.
196-199: 시그니처 포매팅 변경만 — OK문제 없습니다.
app/src/main/java/com/texthip/thip/ui/navigator/navigations/MyPageNavigation.kt (1)
56-58: 새 알림 편집 화면 연동 OK라우트/뒤로가기 처리 일관성 유지되어 보입니다.
app/src/main/java/com/texthip/thip/data/di/ServiceModule.kt (1)
61-64: DI 바인딩 추가 OKRetrofit 서비스 패턴과 일관적입니다.
app/src/main/java/com/texthip/thip/ui/signin/viewmodel/SplashViewModel.kt (1)
49-53: FCM 토큰 전송 로직이 적절히 구현되었습니다.스플래시 화면에서 자동 로그인 시 FCM 토큰을 전송하는 로직이 올바르게 추가되었습니다. 비동기 처리도 적절합니다.
app/src/main/java/com/texthip/thip/ui/signin/viewmodel/SignupViewModel.kt (1)
160-164: FCM 토큰 전송 로직이 적절히 구현되었습니다.회원가입 완료 후 FCM 토큰을 전송하는 로직이 올바르게 추가되었습니다.
app/src/main/java/com/texthip/thip/ui/signin/viewmodel/LoginViewModel.kt (2)
98-103: FCM 토큰 전송 후 UI 상태를 업데이트하는 로직이 잘 구현되었습니다.기존 사용자 로그인 시 FCM 토큰을 전송한 후 Success 상태로 업데이트하는 로직이 적절합니다.
48-49: 결론: 신규 사용자도 회원가입 완료 후 FCM 토큰을 전송하므로 원래 코멘트는 부정확합니다.SignupViewModel에서 회원가입 직후 sendFcmToken()을 호출하고 내부에서 fcmTokenManager.sendCurrentTokenIfExists()를 실행합니다 (app/src/main/java/com/texthip/thip/ui/signin/viewmodel/SignupViewModel.kt: ~144–163). LoginViewModel의 sendFcmTokenAndUpdateState(response)는 기존 사용자 흐름에 해당합니다.
Likely an incorrect or invalid review comment.
app/src/main/java/com/texthip/thip/data/service/NotificationService.kt (1)
13-28: NotificationService 인터페이스가 잘 정의되었습니다.FCM 토큰 등록, 알림 설정 조회 및 업데이트를 위한 API 엔드포인트가 명확하게 정의되었습니다.
app/src/main/java/com/texthip/thip/data/repository/UserRepository.kt (2)
90-112: updateProfile 메서드의 에러 처리가 개선되었습니다.try-catch 블록을 사용한 명시적인 에러 처리와
ThipApiFailureException으로의 변환이 적절합니다.
66-68: 메서드 시그니처 포맷팅이 일관되게 적용되었습니다.멀티라인 파라미터 포맷팅이 일관되게 적용되어 가독성이 향상되었습니다.
Also applies to: 115-117
| @Provides | ||
| @Singleton | ||
| fun provideContext(@ApplicationContext context: Context): Context { | ||
| return context | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
무자격(Context) 바인딩 제공은 오용 위험 — 삭제하고 @ApplicationContext 직접 주입 권장
Hilt는 이미 @ApplicationContext Context를 기본 바인딩합니다. 무자격 Context 싱글톤 바인딩은 소비 측에서 Activity/Service 컨텍스트로 오해하거나 스코프 불일치로 누수를 유발할 수 있습니다. 삭제하고 사용처에서 @ApplicationContext를 명시하세요.
제안 diff(이 파일):
- @Provides
- @Singleton
- fun provideContext(@ApplicationContext context: Context): Context {
- return context
- }무자격 Context 주입 사용처 점검 스크립트:
🏁 Script executed:
#!/bin/bash
# 무자격 Context 주입 패턴 탐색
rg -nP --type=kt -C2 $'@Inject\\s+constructor\\((?s).*\\bContext\\b(?!\\s*\\))' \
; rg -nP --type=kt -C2 $'lateinit\\s+var\\s+\\w+\\s*:\\s*Context' \
; rg -nP --type=kt -C2 $'fun\\s+\\w+\\(.*\\bcontext\\s*:\\s*Context\\b'Length of output: 137
🏁 Script executed:
#!/bin/bash
# 무자격 Context 사용처 재검색: .kt 파일에서 'Context' 등장부와 '@ApplicationContext' 유무를 출력
python3 - <<'PY'
import os, re, sys
ignored_dirs = {'.git','build','out','node_modules','.gradle','gradle'}
matches = []
providers = []
for root, dirs, files in os.walk('.'):
dirs[:] = [d for d in dirs if d not in ignored_dirs]
for fname in files:
if not fname.endswith('.kt'):
continue
path = os.path.join(root, fname)
try:
with open(path, 'r', encoding='utf-8') as fh:
lines = fh.readlines()
text = ''.join(lines)
except Exception:
continue
# provider detection: fun provideContext(...) or @Provides ... provideContext
if re.search(r'@Provides[\s\S]{0,200}fun\s+provideContext\s*\(', text) or re.search(r'fun\s+provideContext\s*\([^)]*\)\s*:\s*Context\b', text):
providers.append(path)
for i, line in enumerate(lines):
if 'Context' not in line:
continue
if not re.search(r'\bContext\b', line):
continue
start = max(0, i-3)
end = min(len(lines), i+3)
snippet = ''.join(lines[start:end]).rstrip('\n')
qualified = bool(re.search(r'@ApplicationContext', snippet))
# classify simple kinds
if re.search(r'@Inject\s+constructor|@Inject\s*$|constructor\s*\(', snippet) or re.search(r'class\s+\w+[^\n]*@Inject', snippet):
ctx_kind = 'constructor/primary'
elif re.search(r'\blateinit\s+var\b|\b(var|val)\s+\w+\s*:\s*Context\b', snippet):
ctx_kind = 'property'
elif re.search(r'fun\s+\w+\s*\([^)]*Context\b', snippet):
ctx_kind = 'function-param'
else:
ctx_kind = 'other'
matches.append({
'file': path,
'line': i+1,
'kind': ctx_kind,
'qualified': qualified,
'snippet': snippet
})
# Output results
if providers:
print('-- providers found --')
for p in providers:
print(p)
print('-- context usages --')
if not matches:
print('NO_CONTEXT_USAGES_FOUND')
else:
for m in matches:
status = 'qualified(@ApplicationContext)' if m['qualified'] else 'unqualified'
print(f"{m['file']}:{m['line']} | {m['kind']} | {status}")
print('----')
print(m['snippet'])
print('----')
PYLength of output: 8903
중요: provideContext 삭제 전 무자격 Context 생성자 주입을 @ApplicationContext로 변경하세요
DataStoreModule의 무자격 Context 바인딩을 바로 제거하면 Hilt가 현재 무자격 Context로 주입하는 생성자들이 실패합니다. 아래 생성자 파라미터들을 먼저 @ApplicationContext로 변경한 뒤 제공자 제거를 진행하세요.
- app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt (라인 24):
변경 →private val context: Context→@param:ApplicationContext private val context: Context - app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt (라인 16):
변경 →private val context: Context→@param:ApplicationContext private val context: Context
제안 diff (DataStoreModule.kt) — 적용 시점: 위 수정 완료 후:
- @Provides
- @Singleton
- fun provideContext(@ApplicationContext context: Context): Context {
- return context
- }📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Provides | |
| @Singleton | |
| fun provideContext(@ApplicationContext context: Context): Context { | |
| return context | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/data/di/DataStoreModule.kt around lines
26-30, the module currently provides an unqualified Context which you must not
remove until dependent constructors are annotated; first update the constructors
to request the application Context by changing the constructor parameter in
app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt (around line
24) from "private val context: Context" to annotate it with
@param:ApplicationContext private val context: Context, and in
app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt
(around line 16) do the same change to @param:ApplicationContext private val
context: Context; after both constructors are updated and compile, remove the
provideContext(@ApplicationContext context: Context) provider method from
DataStoreModule.kt.
| private fun fetchAndSendCurrentToken() { | ||
| try { | ||
| FirebaseMessaging.getInstance().token.addOnCompleteListener { task -> | ||
| if (!task.isSuccessful) { | ||
| Log.w("FCM", "Failed to fetch token", task.exception) | ||
| return@addOnCompleteListener | ||
| } | ||
|
|
||
| val token = task.result | ||
| CoroutineScope(Dispatchers.IO).launch { | ||
| saveFcmToken(token) | ||
| sendTokenToServer(token) | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| Log.e("FCM", "Error fetching FCM token", e) | ||
| } | ||
| } |
There was a problem hiding this comment.
Firebase 토큰 가져오기 시 CoroutineScope 사용 문제
CoroutineScope(Dispatchers.IO)를 직접 생성하는 것은 권장되지 않습니다. 이는 수명 주기가 관리되지 않아 메모리 누수의 원인이 될 수 있습니다.
suspend 함수로 변경하고 suspendCancellableCoroutine을 사용하세요:
-private fun fetchAndSendCurrentToken() {
+private suspend fun fetchAndSendCurrentToken() = suspendCancellableCoroutine { continuation ->
try {
FirebaseMessaging.getInstance().token.addOnCompleteListener { task ->
if (!task.isSuccessful) {
Log.w("FCM", "Failed to fetch token", task.exception)
+ continuation.resumeWith(Result.failure(task.exception ?: Exception("Failed to fetch token")))
return@addOnCompleteListener
}
val token = task.result
- CoroutineScope(Dispatchers.IO).launch {
+ continuation.resumeWith(Result.success(token))
+ }
+ } catch (e: Exception) {
+ Log.e("FCM", "Error fetching FCM token", e)
+ continuation.resumeWith(Result.failure(e))
+ }
+}.let { token ->
+ withContext(Dispatchers.IO) {
saveFcmToken(token)
sendTokenToServer(token)
}
- }
- } catch (e: Exception) {
- Log.e("FCM", "Error fetching FCM token", e)
- }
}필요한 import 추가:
import kotlinx.coroutines.suspendCancellableCoroutine
import kotlinx.coroutines.withContext
import kotlin.coroutines.resumeWith🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt around
lines 53 to 70, the method creates a raw CoroutineScope(Dispatchers.IO) which is
unsafe; change the function to a suspend function that uses
suspendCancellableCoroutine to await FirebaseMessaging.getInstance().token
instead of addOnCompleteListener, then call saveFcmToken and sendTokenToServer
inside withContext(Dispatchers.IO) so work runs on the IO dispatcher without
creating an unmanaged scope; also add the suggested imports
(suspendCancellableCoroutine, withContext, resumeWith) to the file.
| val presignedUrlRequest = validImagePairs.map { it.second } | ||
|
|
||
| // 개수 검증 | ||
| if (validImagePairs.size != presignedResponse.presignedUrls.size) { | ||
| throw Exception("Presigned URL count mismatch: expected ${validImagePairs.size}, got ${presignedResponse.presignedUrls.size}") | ||
| } | ||
| val presignedResponse = feedService.getPresignedUrls(presignedUrlRequest) | ||
| .handleBaseResponse() | ||
| .getOrThrow() ?: throw Exception("Failed to get presigned URLs") | ||
|
|
||
| // 개수 검증 | ||
| if (validImagePairs.size != presignedResponse.presignedUrls.size) { | ||
| throw Exception("개수가 올바르지 않습니다: expected ${validImagePairs.size}, got ${presignedResponse.presignedUrls.size}") | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
프리사인드 URL 매핑을 인덱스에 의존 — 상호 식별자 키로 매핑 권장
응답 순서가 서버에서 보장되지 않으면 잘못 업로드될 수 있습니다. 요청/응답 모두에 식별자(예: clientSideId, filename, hash)를 포함해 Map 기반 매핑으로 바꾸는 걸 권장합니다.
예시(개념):
- 요청:
metadata + clientIndex - 응답:
presignedUrls: [{clientIndex, presignedUrl, fileUrl}] - 매핑:
val byIndex = presignedResponse.presignedUrls.associateBy { it.clientIndex }
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt
Show resolved
Hide resolved
| errorMessage = "알림 설정 정보를 가져올 수 없습니다." | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 메시지가 하드코딩됨
에러 메시지가 코드에 직접 하드코딩되어 있어 국제화 및 유지보수에 어려움이 있습니다.
string 리소스를 사용하도록 개선하세요:
+import com.texthip.thip.R
+import android.content.Context
+import dagger.hilt.android.qualifiers.ApplicationContext
@HiltViewModel
class MypageNotificationEditViewModel @Inject constructor(
- private val notificationRepository: NotificationRepository
+ private val notificationRepository: NotificationRepository,
+ @ApplicationContext private val context: Context
) : ViewModel() {
// Line 52-53
- errorMessage = "알림 설정 정보를 가져올 수 없습니다."
+ errorMessage = context.getString(R.string.error_fetch_notification_settings)
// Line 87-88
- errorMessage = "알림 설정을 업데이트할 수 없습니다."
+ errorMessage = context.getString(R.string.error_update_notification_settings)strings.xml에 추가:
<string name="error_fetch_notification_settings">알림 설정 정보를 가져올 수 없습니다.</string>
<string name="error_update_notification_settings">알림 설정을 업데이트할 수 없습니다.</string>Also applies to: 87-88
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/MypageNotificationEditViewModel.kt
around lines 52-53 (and also lines 87-88), hardcoded Korean error messages are
used; replace them with string resource lookups (e.g.,
context.getString(R.string.error_fetch_notification_settings) and
context.getString(R.string.error_update_notification_settings)). Add the two
entries to res/values/strings.xml as suggested, ensure the ViewModel has access
to a Context or Resources (inject or pass Application/Context) to call
getString, and update the errorMessage assignments to use the resource values
instead of literal strings.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt (1)
11-15: 전체 파일을 메모리에 적재하여 업로드(OOM/GC 압박 가능)readBytes()는 대용량 이미지에서 메모리 급증을 유발합니다. File.asRequestBody로 스트리밍 업로드로 바꾸세요.
-import okhttp3.RequestBody.Companion.toRequestBody +import okhttp3.RequestBody.Companion.asRequestBody @@ - val requestBody = tempFile.readBytes().toRequestBody(mimeType.toMediaType()) + val requestBody = tempFile.asRequestBody(mimeType.toMediaType())Also applies to: 56-56
♻️ Duplicate comments (4)
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (2)
118-142: 제한 병렬 업로드 도입 LGTM — 이전 제안 반영 완료세마포어로 3개 동시 업로드,
awaitAll로 예외 전파 처리 모두 적절합니다. 체감 성능 개선 기대됩니다.
121-129: 프리사인드 URL 매핑을 인덱스에 의존 — 식별자 기반 매핑으로 변경 필요서버 응답 순서가 보장되지 않으면 잘못된 URL로 업로드될 위험이 있습니다. 요청/응답에
clientIndex(또는 filename/hash 등 상호 식별자)를 포함하고 Map 기반으로 매핑하세요. (이전 코멘트와 동일 이슈)아래는 응답에
clientIndex가 포함된다는 가정 하의 개념 예시입니다.@@ - // 개수 검증 - if (validImagePairs.size != presignedResponse.presignedUrls.size) { - throw Exception("개수가 올바르지 않습니다: expected ${validImagePairs.size}, got ${presignedResponse.presignedUrls.size}") - } + // 개수 검증 + if (validImagePairs.size != presignedResponse.presignedUrls.size) { + throw Exception("개수가 올바르지 않습니다: expected ${validImagePairs.size}, got ${presignedResponse.presignedUrls.size}") + } + // 응답 순서 비보장 대비: clientIndex로 매핑 (요청 시 같은 clientIndex 포함 필요) + val byIndex = presignedResponse.presignedUrls.associateBy { it.clientIndex } @@ - validImagePairs.mapIndexed { index, (uri, _) -> + validImagePairs.mapIndexed { index, (uri, _) -> async { semaphore.withPermit { - val presignedInfo = presignedResponse.presignedUrls[index] + val presignedInfo = byIndex[index] + ?: error("presigned url not found for clientIndex=$index")서버 스펙 변경이 어렵다면 최소한 서버에서 응답 순서 보장을 문서화하고, 불일치 감지 로깅/모니터링을 추가해 주세요.
app/src/main/java/com/texthip/thip/utils/auth/DeviceUtils.kt (1)
19-35: ANDROID_ID 제거 및 앱‑스코프 ID 전환: 잘하셨습니다FID 1차, 로컬 UUID 2차 구조로 정책/프라이버시 리스크를 해소했습니다. 이전 코멘트의 우려 사항을 충족합니다. 다만 Play Console Data safety(“Device or other IDs”) 갱신 여부를 확인해 주세요.
다음 스크립트로 레거시 참조가 남아있는지 점검해 주세요:
#!/bin/bash # ANDROID_ID 직접 접근 및 getAndroidDeviceId 호출 잔존 여부 확인 rg -n 'Settings\.Secure\.ANDROID_ID|getAndroidDeviceId\(' -S -g '!**/build/**'app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt (1)
20-33: 에러 처리 일관성 정비: OKregisterFcmToken도 getOrNull()로 통일되어 이전 리뷰 피드백과 일치합니다.
🧹 Nitpick comments (16)
app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt (1)
38-46: 기존 호출부(위치 기반 인자) 잠재적 호환성 리스크새 파라미터가
onValueChange앞에 추가되어, 드물게 위치 기반 인자로 호출한 곳이 있었다면 컴파일 오류가 날 수 있습니다. 현재 모듈은 빌드가 통과했을 가능성이 높지만, 안전망으로 구버전 시그니처 오버로드를 추가하면 리스크가 0이 됩니다.권장 오버로드(구 시그니처 유지):
+@Composable +fun SearchBookTextField( + modifier: Modifier = Modifier, + text: String, + hint: String, + onValueChange: (String) -> Unit, + onSearch: (String) -> Unit = {} +) = SearchBookTextField( + modifier = modifier, + text = text, + hint = hint, + backgroundColor = colors.DarkGrey, + onValueChange = onValueChange, + onSearch = onSearch +)또는, 향후 파라미터 추가 시에는 비-람다 파라미터를 가능한 한 뒤쪽(람다 앞)으로 두거나, 처음부터 오버로드를 병행해 두는 방식을 권장합니다.
app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt (5)
26-39: 대용량/저속 네트워크 대비 callTimeout 추가 권장연결/쓰기/읽기 타임아웃이 10초는 보수적입니다. 전체 호출 상한을 두어 장시간 정체를 방지/허용 균형을 맞추는 것이 안전합니다.
private val s3Client = OkHttpClient.Builder() .connectTimeout(10, java.util.concurrent.TimeUnit.SECONDS) .writeTimeout(10, java.util.concurrent.TimeUnit.SECONDS) .readTimeout(10, java.util.concurrent.TimeUnit.SECONDS) + .callTimeout(60, java.util.concurrent.TimeUnit.SECONDS)
31-37: 디버그 로깅에 presigned URL 노출 가능성Level.BASIC은 URL을 로그에 남겨 서명 파라미터가 노출됩니다(디버그 빌드라도 공유 시 유출 위험). 해당 Helper에는 로깅 제외 또는 URL 마스킹을 권장합니다.
47-47: 임시파일 생성은 createTempFile 사용이 안전수동 파일명은 드물게 충돌 여지가 있습니다. cacheDir에 createTempFile 사용을 권장합니다.
- val tempFile = File(context.cacheDir, "temp_image_${System.currentTimeMillis()}") + val tempFile = File.createTempFile("image_", null, context.cacheDir)
75-87: runCatching 내부의 return@withContext는 제어 흐름을 복잡하게 만듦가드( MIME/확장자/사이즈 체크 )는 runCatching 밖에서 처리하고, 필요시 try/catch만 두는 편이 가독성이 좋습니다. 동작 동일합니다.
예시:
suspend fun getImageMetadata(uri: Uri): ImageMetadata? = withContext(Dispatchers.IO) { val mimeType = context.contentResolver.getType(uri) ?: return@withContext null val extension = when (mimeType) { "image/png" -> "png" "image/jpeg", "image/jpg" -> "jpg" "image/gif" -> "gif" else -> return@withContext null } val size = getFileSize(uri) ?: return@withContext null ImageMetadata(extension = extension, size = size) }
97-104: 파일 크기 0 바이트 처리 정책 확인SIZE가 0이면 null을 반환합니다. 0바이트 파일을 에러로 간주한다면 OK, 아니라면 0 허용 또는 명확한 에러 분기(예: Result.Failure)로 처리하는 것이 의도 전달에 좋습니다.
Also applies to: 108-113
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (4)
38-40: 동시 업로드 상수 노출 방식 검토
MAX_CONCURRENT_UPLOADS를BuildConfig또는 원격 설정으로 조정 가능하게 하면 운영 중 트래픽/단말 성능에 맞춘 튜닝이 쉬워집니다.
129-142: 불필요한 정렬/인덱스 페어 제거로 간소화 가능
awaitAll()은 입력 Defered 리스트의 순서를 보장합니다. 인덱스-URL 페어 생성/정렬을 제거해 비용을 줄일 수 있습니다.- imageUploadHelper.uploadImageToS3( - uri = uri, - presignedUrl = presignedInfo.presignedUrl - ).fold( - onSuccess = { - index to presignedInfo.fileUrl // 인덱스와 URL을 함께 반환 - }, - onFailure = { exception -> - throw Exception("Failed to upload image ${index + 1}: ${exception.message}") - } - ) + imageUploadHelper + .uploadImageToS3(uri = uri, presignedUrl = presignedInfo.presignedUrl) + .getOrThrow() + presignedInfo.fileUrl } } - }.awaitAll() - .sortedBy { it.first } // 원래 순서대로 정렬 - .map { it.second } // URL만 추출 + }.awaitAll()
129-135: 에러 메시지에 식별 정보 포함(URI/파일명/원인 예외 체인)오류 트리아지를 위해
uri(또는 파일명)와 원인 예외를 함께 포함해 주세요.- onFailure = { exception -> - throw Exception("Failed to upload image ${index + 1}: ${exception.message}") - } + onFailure = { exception -> + throw Exception("Failed to upload image (index=$index, uri=$uri): ${exception.message}", exception) + }
97-104: 메타데이터 수집 동시성은 과도할 수 있음 — 단순 순차/경량 제한 권장이미지 수가 많지 않다면 순차 처리로도 충분합니다. 대량 업로드만 고려 시 동일 세마포어를 재사용하세요. 간단안:
- val validImagePairs = imageUris.map { uri -> - async { - imageUploadHelper.getImageMetadata(uri)?.let { metadata -> - uri to metadata - } - } - }.awaitAll().filterNotNull() + val validImagePairs = imageUris.mapNotNull { uri -> + imageUploadHelper.getImageMetadata(uri)?.let { metadata -> + uri to metadata + } + }app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/DeleteAccountViewModel.kt (2)
88-95: Google 로그아웃 비동기 완료 대기 누락(성공 로그가 조기 출력됨)signOut()는 Task 반환으로 즉시 리턴합니다. await()로 완료를 보장해 주세요.
- GoogleSignIn.getClient(context, gso).signOut() - Log.d("DeleteAccountViewModel", "구글 로그아웃 성공") + GoogleSignIn.getClient(context, gso).signOut().await() + Log.d("DeleteAccountViewModel", "구글 로그아웃 성공")추가로 계정 탈퇴 시에는 revokeAccess()가 더 적합할 수 있습니다(토큰 무효화). 필요 시 교체/추가 고려 바랍니다.
- 외부 import 추가(파일 상단):
import kotlinx.coroutines.tasks.await
103-112: suspendCancellableCoroutine 취소 안전성 보강취소된 후 resume 호출 가능성을 차단하고, 콜백이 한 번만 완료되도록 방어해 주세요.
- private suspend fun unlinkKakaoAccount() = suspendCancellableCoroutine<Unit> { continuation -> - UserApiClient.instance.unlink { error -> - if (error != null) { - continuation.resume(Unit) // 실패해도 정상적으로 완료 처리 - Log.w("DeleteAccountViewModel", "카카오 연결 해제 실패하지만 계속 진행", error) - } else { - continuation.resume(Unit) - } - } - } + private suspend fun unlinkKakaoAccount() = suspendCancellableCoroutine<Unit> { cont -> + val complete: (Throwable?) -> Unit = { error -> + if (cont.isActive) { + cont.resume(Unit) + } + if (error != null) { + Log.w("DeleteAccountViewModel", "카카오 연결 해제 실패하지만 계속 진행", error) + } + } + cont.invokeOnCancellation { /* no-op: Kakao SDK는 별도 취소 API 없음 */ } + UserApiClient.instance.unlink { error -> complete(error) } + }app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt (2)
25-33: 하드코딩된 플랫폼 문자열 상수화"ANDROID" 매직 스트링을 상수로 올려 오탈자/변경 비용을 줄이세요.
@Singleton class NotificationRepository @Inject constructor( @@ ) { + private companion object { + const val PLATFORM_ANDROID = "ANDROID" + } suspend fun registerFcmToken( deviceId: String, fcmToken: String ): Result<Unit?> { return runCatching { val request = FcmTokenRequest( deviceId = deviceId, fcmToken = fcmToken, - platformType = "ANDROID" + platformType = PLATFORM_ANDROID )
20-33: 일관성 API 제공(선택): deviceId 내부 획득 오버로드 추가다른 메서드들과 대칭을 맞추기 위해 deviceId를 내부에서 조회하는 오버로드를 하나 제공하면 호출부가 단순해집니다(기존 시그니처는 유지).
suspend fun registerFcmToken(fcmToken: String): Result<Unit?> = runCatching { val deviceId = context.getAppScopeDeviceId() val request = FcmTokenRequest(deviceId = deviceId, fcmToken = fcmToken, platformType = PLATFORM_ANDROID) notificationService.registerFcmToken(request).handleBaseResponse().getOrNull() }app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt (2)
44-55: 토큰 내용 검증(공백 방지) 소소한 보강fetch에서 공백 체크를 하지만, 방어적으로 저장/전송 전에도 공백일 경우 스킵하는 가드를 두면 좋습니다.
try { val token = fetchCurrentToken() - saveFcmToken(token) - sendTokenToServer(token) + if (token.isNotBlank()) { + saveFcmToken(token) + sendTokenToServer(token) + } else { + Log.w("FCM", "Fetched blank token, skipping") + } } catch (e: Exception) {
1-110: 토큰 삭제/로그아웃 시나리오 엔드포인트 연계PR 설명에 “토큰 삭제 미완료”가 있어 매니저에 삭제 유틸 추가를 제안합니다. 저장 토큰 삭제와 서버 언레지스터를 한 번에 처리하세요.
예시 구현(새 함수 추가):
suspend fun unregisterAndClearToken() { val token = getFcmTokenOnce() ?: return val deviceId = context.getAppScopeDeviceId() runCatching { notificationRepository.unregisterFcmToken(deviceId, token) }.onSuccess { dataStore.edit { it.remove(FCM_TOKEN_KEY) } Log.d("FCM", "Token unregistered and cleared") }.onFailure { e -> Log.e("FCM", "Failed to unregister token", e) } }unregisterFcmToken API 명세/멱등성 여부 확인 부탁드립니다. 필요하면 초안 PR 드릴게요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
app/src/main/AndroidManifest.xml(2 hunks)app/src/main/java/com/texthip/thip/MainActivity.kt(4 hunks)app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/notification/request/FcmTokenDeleteRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt(6 hunks)app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/data/service/NotificationService.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/DeleteAccountViewModel.kt(3 hunks)app/src/main/java/com/texthip/thip/utils/auth/DeviceUtils.kt(1 hunks)app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt(6 hunks)app/src/main/java/com/texthip/thip/utils/permission/NotificationPermissionUtils.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/texthip/thip/MainActivity.kt
- app/src/main/java/com/texthip/thip/data/service/NotificationService.kt
- app/src/main/AndroidManifest.xml
🔇 Additional comments (16)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupBookSearchBottomSheet.kt (1)
83-91: 검색 필드 배경색 명시 전달 좋아요BottomSheet 맥락에 맞춰 색을 오버라이드한 의도가 명확합니다. 시각적 일관성 측면에서도 무리 없어 보입니다. 다만 DarkGrey02 배경 + White 텍스트 대비가 충분한지 한번만 확인 부탁드립니다.
app/src/main/java/com/texthip/thip/ui/common/forms/SearchBookTextField.kt (1)
43-46: backgroundColor 파라미터 추가 적절기본값을 두고 Box 배경에 바로 적용한 변경이 명확합니다. 기존 기본 동작도 유지됩니다.
Also applies to: 57-58
app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt (1)
8-8: @param:ApplicationContext 사용 적절 — DI 일관성만 확인생성자 주입에서 @param 타깃 지정은 문제 없습니다. 프로젝트 전반에서 동일 패턴(@ApplicationContext 또는 @param:ApplicationContext)을 일관 적용되는지 확인 부탁드립니다.
Also applies to: 23-24
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (5)
113-116: 개수 검증 추가 좋습니다프리사인드 URL 개수 검증이 있어 조기 실패 처리 가능해졌습니다. LGTM.
153-159: 시그니처 포매팅 변경만 — OK동작 변경 없음. 가독성 향상에 동의합니다.
180-186: 시그니처 포매팅 변경만 — OK동작 변경 없음. 일관성 유지되어 좋습니다.
209-215: 시그니처 포매팅 변경만 — OK동작 변경 없음. 문제 없습니다.
12-13: kotlinx-coroutines 버전(Semaphore/withPermit 지원) 확인 필요gradle/libs.versions.toml 및 build.gradle.kts(app 포함)을 검색했으나 kotlinx-coroutines 의존성/버전 선언을 찾을 수 없습니다. Semaphore/withPermit은 kotlinx-coroutines 1.7.x 이상에서 안정 권장이므로 의존성 선언 위치에서 버전을 확인하거나 >=1.7.x로 명시하세요.
app/src/main/java/com/texthip/thip/data/model/notification/request/FcmTokenDeleteRequest.kt (1)
5-8: 스키마 명 확인 요청(서버와 필드명 매칭 검증)deviceId 단일 필드 설계는 간결합니다. 서버 계약서에서 필드명이 deviceId인지(스네이크/카멜 케이스 등)와 추가 파라미터(앱 버전/플랫폼 등) 요구 여부를 한번 더 확인해 주세요.
app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/DeleteAccountViewModel.kt (1)
32-34: DI 확장 OKNotificationRepository 의존성 주입 추가로 토큰 삭제 선행이 가능해졌습니다. 구성 좋습니다.
app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt (6)
22-31: Hilt 싱글톤/주입 구성 적절DI 경계, DataStore 주입, 키 상수화 모두 무난합니다.
33-42: 동일 토큰(no-op) 시 재등록 경로 필요 여부 확인서버 측 등록 실패(네트워크 오류 등) 또는 알림 권한이 나중에 허용된 경우, 저장 토큰 == 신규 토큰이면 전송이 스킵됩니다. 권한 허용 이벤트 또는 앱 재진입 시 재등록을 보장할 별도 트리거(예: 권한 허용 콜백에서 sendCurrentTokenIfExists 호출)가 있는지 확인해 주세요. 없으면 재전송 플래그/상태 저장이 필요할 수 있습니다.
44-59: 현재 토큰 전송 트리거 지점 점검앱 시작/로그인/권한 허용 직후 등 실사용 시점에서 반드시 호출되는지 확인 필요합니다. 특히 저장 토큰이 존재하지 않아 fetch 경로를 타는 경우, 무한 대기 방지를 위해 타임아웃을 두는 것도 권장합니다(아래 fetch 리팩터 참고).
95-101: 권한 미허용 시 등록 스킵 정책 재검토일반적으로 토큰 등록은 알림 권한과 분리해 선행해도 무방합니다(권한은 표시 행위에만 영향). 현재 정책대로면 권한을 허용하기 전까지 서버가 토큰을 알 수 없어 초기 푸시 시나리오(예: 온보딩 후 프로모션)가 막힐 수 있습니다. 제품 정책에 맞는지 확인하고, 필요 시 “토글=수신 여부”는 서버 플래그로 분리하고 토큰은 항상 등록하도록 변경을 권장합니다. API 33 미만에서의 권한 분기 처리도 함께 확인해 주세요.
102-110: 네트워크 신뢰성 향상: 재시도(지수 백오프) 적용 제안 — 검증 필요app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt (라인 102–110)을 레포에서 찾을 수 없어 자동 확인 불가. 다음을 권장합니다:
- registerFcmToken(deviceId, token) 호출에 대해 제한적 재시도(예: 3회, 지수 백오프) 적용.
- 서버가 동일(deviceId, token) 등록에 대해 멱등성(중복 등록 무해)을 제공하는지 확인.
61-84: Task-코루틴 브리지 개선: await + withTimeout 사용 제안 (의존성 확인 필요)suspendCancellableCoroutine 수동 브리지는 취소/리스너 해제 관리에서 오류 소지가 큽니다. Firebase Task에 대해 kotlinx.coroutines.tasks.await + withTimeout으로 간결히 교체하세요.
위치: app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt (fetchCurrentToken)
권장 교체(diff):
- private suspend fun fetchCurrentToken(): String = suspendCancellableCoroutine { continuation -> - try { - FirebaseMessaging.getInstance().token.addOnCompleteListener { task -> - when { - task.isSuccessful -> { - val token = task.result - if (token != null) { - continuation.resume(token) - } else { - continuation.resumeWithException(IllegalStateException("FCM token is null")) - } - } - else -> { - val exception = task.exception ?: Exception("Unknown error fetching FCM token") - Log.w("FCM", "Failed to fetch token", exception) - continuation.resumeWithException(exception) - } - } - } - } catch (e: Exception) { - Log.e("FCM", "Error fetching FCM token", e) - continuation.resumeWithException(e) - } - } + private suspend fun fetchCurrentToken(): String = withTimeout(10_000) { + try { + val token = FirebaseMessaging.getInstance().token.await() + require(!token.isNullOrBlank()) { "FCM token is null or blank" } + token + } catch (e: Exception) { + Log.w("FCM", "Failed to fetch token", e) + throw e + } + }필요 import 변경:
- import kotlinx.coroutines.suspendCancellableCoroutine - import kotlin.coroutines.resume - import kotlin.coroutines.resumeWithException + import kotlinx.coroutines.tasks.await + import kotlinx.coroutines.withTimeout검증/조치: 레포 검색 결과 gradle/libs.versions.toml 및 app/build.gradle.kts에서 'org.jetbrains.kotlinx:kotlinx-coroutines-play-services' 의존성이 발견되지 않았습니다. (참고: app/src/.../DeviceUtils.kt는 kotlinx.coroutines.tasks.await를 import하고 있음) 의존성 존재 여부를 확인하거나 libs.versions.toml 또는 app 모듈에 kotlinx-coroutines-play-services 의존성을 추가하세요.
| private suspend fun performLocalDataCleanup(context: Context) { | ||
| runCatching { | ||
| // 1. 토큰과 디바이스 데이터 정리 | ||
| tokenManager.clearTokens() | ||
| context.clearAppScopeDeviceData() | ||
|
|
||
| // 2. 카카오 SDK 연결 끊기 (동기화 보장) | ||
| runCatching { | ||
| unlinkKakaoAccount() | ||
| Log.d("DeleteAccountViewModel", "카카오 연결 끊기 성공") | ||
| }.onFailure { e -> | ||
| Log.e("DeleteAccountViewModel", "카카오 연결 끊기 실패", e) | ||
| } | ||
|
|
||
| // 3. 구글 SDK 로그아웃 | ||
| runCatching { | ||
| val gso = GoogleSignInOptions.Builder(GoogleSignInOptions.DEFAULT_SIGN_IN).build() | ||
| GoogleSignIn.getClient(context, gso).signOut() | ||
| Log.d("DeleteAccountViewModel", "구글 로그아웃 성공") | ||
| }.onFailure { e -> | ||
| Log.e("DeleteAccountViewModel", "구글 로그아웃 실패", e) | ||
| } | ||
|
|
||
| Log.i("DeleteAccountViewModel", "로컬 데이터 정리 완료") | ||
| }.onFailure { e -> | ||
| Log.e("DeleteAccountViewModel", "로컬 데이터 정리 중 오류", e) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
로컬 FCM 토큰/FID 삭제까지 포함하면 프라이버시 일관성이 향상됩니다
서버측 토큰 삭제 + 로컬 FCM 토큰 삭제 + FID 삭제를 함께 수행해 주시면 재가입/재설치 시 상태가 깔끔합니다.
private suspend fun performLocalDataCleanup(context: Context) {
runCatching {
// 1. 토큰과 디바이스 데이터 정리
tokenManager.clearTokens()
context.clearAppScopeDeviceData()
// 2. 카카오 SDK 연결 끊기 (동기화 보장)
@@
// 3. 구글 SDK 로그아웃
runCatching {
val gso = GoogleSignInOptions.Builder(GoogleSignInOptions.DEFAULT_SIGN_IN).build()
- GoogleSignIn.getClient(context, gso).signOut()
+ // 탈퇴 시 토큰 무효화까지 필요하면 revokeAccess().await() 사용 검토
+ GoogleSignIn.getClient(context, gso).signOut().await()
Log.d("DeleteAccountViewModel", "구글 로그아웃 성공")
}.onFailure { e ->
Log.e("DeleteAccountViewModel", "구글 로그아웃 실패", e)
}
Log.i("DeleteAccountViewModel", "로컬 데이터 정리 완료")참고: clearAppScopeDeviceData()에서 FCM 토큰/FID 삭제를 포함하도록 변경하면 여기서는 호출만으로 일괄 처리됩니다(권장).
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private suspend fun performLocalDataCleanup(context: Context) { | |
| runCatching { | |
| // 1. 토큰과 디바이스 데이터 정리 | |
| tokenManager.clearTokens() | |
| context.clearAppScopeDeviceData() | |
| // 2. 카카오 SDK 연결 끊기 (동기화 보장) | |
| runCatching { | |
| unlinkKakaoAccount() | |
| Log.d("DeleteAccountViewModel", "카카오 연결 끊기 성공") | |
| }.onFailure { e -> | |
| Log.e("DeleteAccountViewModel", "카카오 연결 끊기 실패", e) | |
| } | |
| // 3. 구글 SDK 로그아웃 | |
| runCatching { | |
| val gso = GoogleSignInOptions.Builder(GoogleSignInOptions.DEFAULT_SIGN_IN).build() | |
| GoogleSignIn.getClient(context, gso).signOut() | |
| Log.d("DeleteAccountViewModel", "구글 로그아웃 성공") | |
| }.onFailure { e -> | |
| Log.e("DeleteAccountViewModel", "구글 로그아웃 실패", e) | |
| } | |
| Log.i("DeleteAccountViewModel", "로컬 데이터 정리 완료") | |
| }.onFailure { e -> | |
| Log.e("DeleteAccountViewModel", "로컬 데이터 정리 중 오류", e) | |
| } | |
| } | |
| private suspend fun performLocalDataCleanup(context: Context) { | |
| runCatching { | |
| // 1. 토큰과 디바이스 데이터 정리 | |
| tokenManager.clearTokens() | |
| context.clearAppScopeDeviceData() | |
| // 2. 카카오 SDK 연결 끊기 (동기화 보장) | |
| runCatching { | |
| unlinkKakaoAccount() | |
| Log.d("DeleteAccountViewModel", "카카오 연결 끊기 성공") | |
| }.onFailure { e -> | |
| Log.e("DeleteAccountViewModel", "카카오 연결 끊기 실패", e) | |
| } | |
| // 3. 구글 SDK 로그아웃 | |
| runCatching { | |
| val gso = GoogleSignInOptions.Builder(GoogleSignInOptions.DEFAULT_SIGN_IN).build() | |
| // 탈퇴 시 토큰 무효화까지 필요하면 revokeAccess().await() 사용 검토 | |
| GoogleSignIn.getClient(context, gso).signOut().await() | |
| Log.d("DeleteAccountViewModel", "구글 로그아웃 성공") | |
| }.onFailure { e -> | |
| Log.e("DeleteAccountViewModel", "구글 로그아웃 실패", e) | |
| } | |
| Log.i("DeleteAccountViewModel", "로컬 데이터 정리 완료") | |
| }.onFailure { e -> | |
| Log.e("DeleteAccountViewModel", "로컬 데이터 정리 중 오류", e) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/DeleteAccountViewModel.kt
around lines 74-101, the local cleanup currently clears tokens and device data
but does not ensure local FCM token and Firebase Installations ID (FID) are
removed; update the cleanup to also delete the FCM token and FID (either by
adding those operations into clearAppScopeDeviceData() as recommended, or by
calling FirebaseMessaging.getInstance().deleteToken() and
FirebaseInstallations.getInstance().delete() here), wrap those calls in
runCatching blocks with appropriate success/error Log statements, and ensure
calls are awaited in this suspend function so deletion completes before
finishing.
| import com.google.firebase.installations.FirebaseInstallations | ||
| import kotlinx.coroutines.flow.first | ||
| import kotlinx.coroutines.flow.map | ||
| import kotlinx.coroutines.tasks.await | ||
| import java.util.UUID |
There was a problem hiding this comment.
🛠️ Refactor suggestion
회원탈퇴/데이터 정리 시 로컬 FID/FCM 토큰도 폐기 권장
서버측 토큰 삭제 외에, 앱 로컬에서도 FCM 토큰과 FID를 삭제하면 프라이버시와 재가입 시 일관성이 좋아집니다.
import com.google.firebase.installations.FirebaseInstallations
+import com.google.firebase.messaging.FirebaseMessaging
@@
suspend fun Context.clearAppScopeDeviceData() {
try {
- appDeviceIdDataStore.edit { preferences ->
- preferences.clear()
- }
- Log.i("DeviceUtils", "App-scope device data cleared successfully")
+ // 1) 로컬 앱-스코프 ID 제거
+ appDeviceIdDataStore.edit { it.clear() }
+ // 2) FCM 토큰 제거
+ runCatching { FirebaseMessaging.getInstance().deleteToken().await() }
+ .onFailure { Log.w("DeviceUtils", "Failed to delete FCM token (ignored)", it) }
+ // 3) Firebase Installation 삭제(FID 초기화)
+ runCatching { FirebaseInstallations.getInstance().delete().await() }
+ .onFailure { Log.w("DeviceUtils", "Failed to delete Firebase Installation (ignored)", it) }
+ Log.i("DeviceUtils", "App-scope device & Firebase installation cleared")
} catch (e: Exception) {
Log.e("DeviceUtils", "Failed to clear app-scope device data", e)
}
}Also applies to: 90-99
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/utils/auth/DeviceUtils.kt around lines
10-14 and 90-99, implement explicit local cleanup of Firebase FID and FCM token
on account deletion: call FirebaseInstallations.getInstance().delete() and
FirebaseMessaging.getInstance().deleteToken()/delete() (as appropriate) from a
coroutine and await the tasks, handling and logging any exceptions; ensure these
calls run after successful server-side deletion so local FID and FCM tokens are
removed for privacy and consistent re-registration.
| private suspend fun Context.getOrCreateLocalDeviceId(): String { | ||
| // 기존 저장된 ID 확인 | ||
| val existingId = appDeviceIdDataStore.data | ||
| .map { preferences -> preferences[APP_DEVICE_ID_KEY] } | ||
| .first() | ||
|
|
||
| return existingId ?: run { | ||
| // 새 UUID 생성 후 저장 | ||
| val newId = UUID.randomUUID().toString() | ||
| appDeviceIdDataStore.edit { preferences -> | ||
| preferences[APP_DEVICE_ID_KEY] = newId | ||
| } | ||
| Log.i("DeviceUtils", "Generated new app-scope device ID") | ||
| newId | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
TOCTOU 제거: UUID 생성/저장을 단일 edit 트랜잭션으로 원자화
현재는 read(.first) 후 write(edit)로 분리되어 동시 호출 시 서로 다른 UUID가 생성/반환될 수 있습니다. 단일 edit 블록에서 “없으면 생성” 패턴으로 원자화해 주세요.
- private suspend fun Context.getOrCreateLocalDeviceId(): String {
- // 기존 저장된 ID 확인
- val existingId = appDeviceIdDataStore.data
- .map { preferences -> preferences[APP_DEVICE_ID_KEY] }
- .first()
-
- return existingId ?: run {
- // 새 UUID 생성 후 저장
- val newId = UUID.randomUUID().toString()
- appDeviceIdDataStore.edit { preferences ->
- preferences[APP_DEVICE_ID_KEY] = newId
- }
- Log.i("DeviceUtils", "Generated new app-scope device ID")
- newId
- }
- }
+ private suspend fun Context.getOrCreateLocalDeviceId(): String {
+ var result: String? = null
+ appDeviceIdDataStore.edit { prefs ->
+ result = prefs[APP_DEVICE_ID_KEY]
+ ?: UUID.randomUUID().toString().also { newId ->
+ prefs[APP_DEVICE_ID_KEY] = newId
+ Log.i("DeviceUtils", "Generated new app-scope device ID")
+ }
+ }
+ return requireNotNull(result)
+ }
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private suspend fun Context.getOrCreateLocalDeviceId(): String { | |
| // 기존 저장된 ID 확인 | |
| val existingId = appDeviceIdDataStore.data | |
| .map { preferences -> preferences[APP_DEVICE_ID_KEY] } | |
| .first() | |
| return existingId ?: run { | |
| // 새 UUID 생성 후 저장 | |
| val newId = UUID.randomUUID().toString() | |
| appDeviceIdDataStore.edit { preferences -> | |
| preferences[APP_DEVICE_ID_KEY] = newId | |
| } | |
| Log.i("DeviceUtils", "Generated new app-scope device ID") | |
| newId | |
| } | |
| } | |
| private suspend fun Context.getOrCreateLocalDeviceId(): String { | |
| var result: String? = null | |
| appDeviceIdDataStore.edit { prefs -> | |
| result = prefs[APP_DEVICE_ID_KEY] | |
| ?: UUID.randomUUID().toString().also { newId -> | |
| prefs[APP_DEVICE_ID_KEY] = newId | |
| Log.i("DeviceUtils", "Generated new app-scope device ID") | |
| } | |
| } | |
| return requireNotNull(result) | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/utils/auth/DeviceUtils.kt around lines 56
to 71, replace the current read-then-write flow with a single atomic
appDeviceIdDataStore.edit transaction: inside edit check if APP_DEVICE_ID_KEY
exists, if so use that value, otherwise generate a new UUID, store it into
preferences, and use that new value; capture the chosen id in a local variable
outside the edit and return it after edit completes (keep the Log.i message when
a new id is generated).
| val response = s3Client.newCall(request).execute() | ||
|
|
||
| if (!response.isSuccessful) { | ||
| throw Exception("S3 upload failed: ${response.code} ${response.message}") | ||
| } |
There was a problem hiding this comment.
OkHttp Response를 닫지 않아 커넥션 누수 위험
execute()로 받은 Response를 닫지 않아 커넥션 풀 누수가 발생할 수 있습니다. use로 감싸 자동 close하세요.
+import java.io.IOException
@@
- val response = s3Client.newCall(request).execute()
-
- if (!response.isSuccessful) {
- throw Exception("S3 upload failed: ${response.code} ${response.message}")
- }
+ s3Client.newCall(request).execute().use { response ->
+ if (!response.isSuccessful) {
+ throw IOException("S3 upload failed: ${response.code} ${response.message}")
+ }
+ }
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val response = s3Client.newCall(request).execute() | |
| if (!response.isSuccessful) { | |
| throw Exception("S3 upload failed: ${response.code} ${response.message}") | |
| } | |
| import java.io.IOException | |
| s3Client.newCall(request).execute().use { response -> | |
| if (!response.isSuccessful) { | |
| throw IOException("S3 upload failed: ${response.code} ${response.message}") | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/utils/image/ImageUploadHelper.kt around
lines 62 to 66, the OkHttp Response from s3Client.newCall(request).execute() is
not closed which can leak connections; wrap the execute() call in a Kotlin use
block (or explicitly call response.close()) so the Response is closed
automatically, and move the isSuccessful check and any body handling inside that
use block, throwing the Exception if not successful.
| import android.Manifest | ||
| import android.content.Context | ||
| import android.content.pm.PackageManager | ||
| import android.os.Build | ||
| import androidx.core.content.ContextCompat | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
권한(grant) 여부 외에도 시스템 알림 허용 상태 점검 필요
Android 12 이하에서도 사용자가 앱 알림을 꺼둘 수 있어 단순 true 반환은 오해의 소지가 있습니다. NotificationManagerCompat.areNotificationsEnabled() 체크를 보강해 주세요.
import android.os.Build
import androidx.core.content.ContextCompat
+import androidx.core.app.NotificationManagerCompat
@@
- // 알림 권한이 허용되었는지 확인
+ // 알림 권한이 허용되었는지 확인 (SDK 33+ 전용)
fun isNotificationPermissionGranted(context: Context): Boolean {
return if (isNotificationPermissionRequired()) {
ContextCompat.checkSelfPermission(
context,
Manifest.permission.POST_NOTIFICATIONS
) == PackageManager.PERMISSION_GRANTED
} else {
- // Android 12 이하에서는 권한이 자동으로 허용됨
- true
+ // Android 12 이하: 런타임 권한 없음 → 별도 권한 체크 불필요
+ true
}
}
@@
- // 알림 권한 요청이 필요한지 확인
+ // 시스템 레벨에서 실제 알림 허용 상태
+ fun areNotificationsEnabled(context: Context): Boolean =
+ NotificationManagerCompat.from(context).areNotificationsEnabled()
+
+ // 알림 권한 요청이 필요한지 확인
fun shouldRequestNotificationPermission(context: Context): Boolean {
return isNotificationPermissionRequired() && !isNotificationPermissionGranted(context)
}
+
+ // 실제 표시 가능 여부(권한 + 시스템 허용)
+ fun isNotificationDeliveryEnabled(context: Context): Boolean {
+ return if (isNotificationPermissionRequired()) {
+ isNotificationPermissionGranted(context) && areNotificationsEnabled(context)
+ } else {
+ areNotificationsEnabled(context)
+ }
+ }Also applies to: 18-36
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/utils/permission/NotificationPermissionUtils.kt
around lines 3-8 and 18-36, the current logic only returns whether the
POST_NOTIFICATIONS permission is granted (or implicitly true on older Android),
but it must also verify the system-level notification switch; update the
function to first check
NotificationManagerCompat.from(context).areNotificationsEnabled() and return
false if that is false, then for Android 13+ (Build.VERSION.SDK_INT >=
Build.VERSION_CODES.TIRAMISU) additionally check
ContextCompat.checkSelfPermission(context,
Manifest.permission.POST_NOTIFICATIONS) == PackageManager.PERMISSION_GRANTED,
and for older versions rely solely on areNotificationsEnabled(); ensure
NotificationManagerCompat is imported and that the combined result is returned
(system-level enabled AND permission granted when applicable).
Nico1eKim
left a comment
Nico1eKim
left a comment
There was a problem hiding this comment.
와 코드 진짜 복잡하네요 .. 이해하는데 시간 오래걸렸어요 진짜 수고했습니다 🫡👍
네비게이션은 나눠서 구현하면 될까여 ??
There was a problem hiding this comment.
해당 유틸 파일은 지피티가 생성해준 코드인데 디바이스 아이디를 이런식으로 관리한다고 합니다.
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
😢 해결하지 못한 과제
[] 토큰 삭제
📢 리뷰어들에게
화면에 보이지는 않지만 안드 폰에 설치해서 알림 페이지를 띄운 결과 알림이 오는 것은 확인했고, 토큰 등록 부터 알림 수신여부 까지 구현 완료 했습니다. 현재는 알림이 오면 notification을 생성하고 누르면 메인 엑티비티로 이동하게만 구현해 놨습니다.
또 기타 추가 수정사항이나 피그마 보다가 이상한 부분 수정 완료했습니다. (줄바꿈은 하다가 생각하는 겸 했습니다)
Summary by CodeRabbit
신규 기능
개선 사항
UI/스타일