feat: FCM 푸시 알림 환경 설정 - #62
Open
WAcAW9 wants to merge 19 commits into
Open
Conversation
…Android into feat/51-fcm-alert
- runRepositoryCatching 제거 → safeApiResultUnit로 반영
1 task
📝 WalkthroughWalkthroughFirebase Cloud Messaging 의존성과 메시징 서비스를 추가했습니다. FCM 토큰을 로컬 및 서버에 저장하고 인증 상태에 따라 등록·해제합니다. 채팅, 공지 및 수신 전화 payload를 파싱해 Android 알림으로 표시합니다. ChangesFCM 푸시 알림
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant LoginRepositoryImpl
participant PushTokenManager
participant FirebaseFcmTokenProvider
participant PushTokenRepository
participant FcmTokenDataStore
User->>LoginRepositoryImpl: 로그인
LoginRepositoryImpl->>PushTokenManager: registerCurrentDevice()
PushTokenManager->>FirebaseFcmTokenProvider: getToken()
FirebaseFcmTokenProvider-->>PushTokenManager: FCM 토큰
PushTokenManager->>PushTokenRepository: register(token)
PushTokenRepository-->>PushTokenManager: Result<Unit>
PushTokenManager->>FcmTokenDataStore: saveToken(token)
sequenceDiagram
participant Firebase
participant CallFromAiFirebaseMessagingService
participant PushPayloadParser
participant PushNotificationHelper
participant AndroidNotificationManager
Firebase->>CallFromAiFirebaseMessagingService: onMessageReceived(message)
CallFromAiFirebaseMessagingService->>PushPayloadParser: parse(data, title, body)
PushPayloadParser-->>CallFromAiFirebaseMessagingService: PushPayload
CallFromAiFirebaseMessagingService->>PushNotificationHelper: 유형별 알림 표시
PushNotificationHelper->>AndroidNotificationManager: GENERAL 채널 알림 게시
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Gemini AI 코드리뷰안녕하세요, 시니어 Android 개발자입니다. Firebase Cloud Messaging (FCM) 연동과 푸시 알림 기능을 추가하는 이번 PR, 전반적으로 매우 꼼꼼하고 모범적인 구현 사례가 많아 인상 깊습니다. 특히, 아키텍처 원칙과 비즈니스 로직 분리에 신경 쓴 점이 돋보입니다. 자세한 코드 리뷰는 아래와 같습니다. ✅ 전반적인 피드백 (긍정적인 부분)
|
![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 (2)
app/src/main/java/kr/co/call/callfromai/AppViewModel.kt (1)
39-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win로그인 여부 확인 후 토큰 등록 로직이 두 곳에 중복됩니다.
tokenDataStore.getAccessToken().isNullOrBlank()체크와 그에 따른pushTokenManager.registerCurrentDevice()호출이 두 파일에 동일하게 구현되어 있습니다. 근본 원인은 이 가드 로직이PushTokenManager내부가 아니라 각 호출부에 있다는 점입니다.
app/src/main/java/kr/co/call/callfromai/AppViewModel.kt#L39-L45: 이 가드 로직을 제거하고,PushTokenManager에 로그인 여부를 확인하는 진입점(예:registerCurrentDeviceIfLoggedIn())을 추가해 호출하십시오.app/src/main/java/kr/co/call/callfromai/push/CallFromAiFirebaseMessagingService.kt#L34-L40: 동일하게PushTokenManager의 새 진입점을 호출하도록 변경하십시오.이렇게 하면 로그인 가드 로직이 한 곳에만 존재하게 되어, 향후 변경 시 두 곳을 동시에 수정할 필요가 없습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/kr/co/call/callfromai/AppViewModel.kt` around lines 39 - 45, 중복된 로그인 가드를 PushTokenManager 내부로 이동하십시오. AppViewModel.kt의 registerPushTokenIfLoggedIn과 CallFromAiFirebaseMessagingService.kt의 해당 토큰 등록 호출을 새 진입점(예: registerCurrentDeviceIfLoggedIn())으로 변경하고, PushTokenManager에서 access token 확인 후 로그인 상태일 때만 registerCurrentDevice()를 호출하도록 구현하십시오.core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt (1)
51-53: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winFCM 토큰 등록이 로그인 완료를 지연시킵니다.
pushTokenManager.registerCurrentDevice()는 네트워크 호출을 포함하는 suspend 함수입니다. JWT는 이미 저장되었으므로, 이 호출의 완료 여부는 로그인 결과에 영향을 주지 않습니다. 하지만 현재 코드는 이 호출이 끝날 때까지LoginToken반환을 지연시킵니다. 이는 로그인 완료까지 걸리는 시간을 불필요하게 늘립니다.
registerCurrentDevice()를 별도 스코프(예: 애플리케이션 스코프)에서 fire-and-forget 방식으로 실행하는 것을 권장합니다.♻️ 제안하는 리팩터링 방향
- // AuthInterceptor 가 JWT 를 쓸 수 있는 상태에서 등록 - pushTokenManager.registerCurrentDevice() + // AuthInterceptor 가 JWT 를 쓸 수 있는 상태에서 등록 (로그인 응답을 지연시키지 않도록 비동기 실행) + applicationScope.launch { pushTokenManager.registerCurrentDevice() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt` around lines 51 - 53, Update the login completion flow around pushTokenManager.registerCurrentDevice() so the suspend network call runs asynchronously in an application-level or equivalent independent scope. Return LoginToken without awaiting token registration, while preserving the existing JWT persistence and registration trigger behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/kr/co/call/callfromai/MainActivity.kt`:
- Line 36: 알림 권한 요청 완료 상태를 영속 저장하도록 requestNotificationPermissionIfNeeded()와
shouldRequest() 흐름을 수정하세요. 최초 요청 여부와 사용자 거부를 구분해 onCreate() 재실행마다 자동 요청되지 않게 하고,
거부 후 재요청은 앱 설정 화면 등 명시적인 사용자 동작에서만 수행되도록 보장하세요.
In `@app/src/main/java/kr/co/call/callfromai/push/FirebaseFcmTokenProvider.kt`:
- Around line 17-23: The catch block in getToken() currently catches all
Exceptions including CancellationException, which suppresses coroutine
cancellation signals. Add a separate catch block for CancellationException
before the existing catch (e: Exception) that immediately re-throws it, then
preserve the existing Exception handler that logs the error and returns null for
actual Firebase token failures.
In `@app/src/main/java/kr/co/call/callfromai/push/PushNotificationHelper.kt`:
- Around line 12-95: 알림에 탭 동작이 연결되지 않았습니다. PushNotificationHelper의 showChat과
showCall에서 각각 chatRoomId와 callId/characterName을 포함한 명시적 PendingIntent를 생성해
notify 호출에 전달하고, notify의 NotificationCompat.Builder에 setContentIntent를 설정하십시오.
채팅 알림은 해당 채팅방으로 이동하게 하고, 통화 알림은 최소한 채팅방 이동을 제공하며 기존 PushNotificationExtras 키를
재사용해 수신 화면이 필요한 데이터를 받을 수 있도록 하십시오.
- Around line 97-101: Update chatNotificationId in PushNotificationHelper so
chat notification IDs use a dedicated range that cannot overlap
NOTICE_NOTIFICATION_ID or the range produced by callNotificationId. Apply a
consistent offset or range mapping while preserving deterministic, valid Int IDs
for chat rooms.
In `@core/data/src/main/java/kr/co/call/data/push/PushTokenManager.kt`:
- Around line 50-60: Update unregisterCurrentDevice so local FCM/session data is
not cleared when repository deletion fails; preserve the authentication state or
persist an explicit pending-deletion state for retry/cancellation, and clear
tokens only after successful deletion. Coordinate MyPageRepositoryImpl.logout
with this outcome so JWT credentials remain available for the required retry
policy. Serialize deletion and registration within the same session lifecycle,
or make CallFromAiFirebaseMessagingService.onNewToken block
registerCurrentDevice during and after logout to prevent a post-logout
re-registration.
- Around line 30-60: Handle FcmTokenDataStore write failures in
registerCurrentDevice and unregisterCurrentDevice by wrapping saveToken and
clearToken in try-catch blocks, logging non-cancellation exceptions, and
allowing the methods to return without propagating them. Always rethrow
CancellationException; keep PushTokenRepository Result handling unchanged.
In `@core/datastore/src/main/java/kr/co/call/datastore/FcmTokenDataStore.kt`:
- Around line 50-54: Update saveToken so the FCM token is encrypted through the
project’s established SecretBox or equivalent encryption layer before writing to
FCM_TOKEN, and add the required module dependency/imports. Ensure existing
plaintext fcm_token values are migrated to encrypted form when read or during
initialization, preserving token retrieval behavior.
In `@core/domain/src/main/java/kr/co/call/domain/model/push/PushPayload.kt`:
- Around line 14-22: Update PushPayloadParser.parseChat to read title and body
from the data map using new PushDataKeys.TITLE and PushDataKeys.BODY constants,
while preserving notificationTitle/notificationBody as fallbacks when data
values are absent; ensure data-only CHAT messages populate Chat.title and
Chat.body correctly.
---
Nitpick comments:
In `@app/src/main/java/kr/co/call/callfromai/AppViewModel.kt`:
- Around line 39-45: 중복된 로그인 가드를 PushTokenManager 내부로 이동하십시오. AppViewModel.kt의
registerPushTokenIfLoggedIn과 CallFromAiFirebaseMessagingService.kt의 해당 토큰 등록 호출을
새 진입점(예: registerCurrentDeviceIfLoggedIn())으로 변경하고, PushTokenManager에서 access
token 확인 후 로그인 상태일 때만 registerCurrentDevice()를 호출하도록 구현하십시오.
In
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt`:
- Around line 51-53: Update the login completion flow around
pushTokenManager.registerCurrentDevice() so the suspend network call runs
asynchronously in an application-level or equivalent independent scope. Return
LoginToken without awaiting token registration, while preserving the existing
JWT persistence and registration trigger behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c4de18b3-ce5d-4e94-a8ac-092968c1b846
📒 Files selected for processing (26)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/kr/co/call/callfromai/App.ktapp/src/main/java/kr/co/call/callfromai/AppViewModel.ktapp/src/main/java/kr/co/call/callfromai/MainActivity.ktapp/src/main/java/kr/co/call/callfromai/di/PushModule.ktapp/src/main/java/kr/co/call/callfromai/notification/NotificationChannels.ktapp/src/main/java/kr/co/call/callfromai/notification/NotificationPermission.ktapp/src/main/java/kr/co/call/callfromai/push/CallFromAiFirebaseMessagingService.ktapp/src/main/java/kr/co/call/callfromai/push/FirebaseFcmTokenProvider.ktapp/src/main/java/kr/co/call/callfromai/push/PushNotificationHelper.ktbuild.gradle.ktscore/data/src/main/java/kr/co/call/data/di/RepositoryModule.ktcore/data/src/main/java/kr/co/call/data/push/PushTokenManager.ktcore/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.ktcore/data/src/main/java/kr/co/call/data/repositoryImpl/MyPageRepositoryImpl.ktcore/data/src/main/java/kr/co/call/data/repositoryImpl/PushTokenRepositoryImpl.ktcore/datastore/src/main/java/kr/co/call/datastore/FcmTokenDataStore.ktcore/domain/src/main/java/kr/co/call/domain/model/push/PushPayload.ktcore/domain/src/main/java/kr/co/call/domain/model/push/PushPayloadParser.ktcore/domain/src/main/java/kr/co/call/domain/push/FcmTokenProvider.ktcore/domain/src/main/java/kr/co/call/domain/repository/PushTokenRepository.ktcore/network/src/main/java/kr/co/call/network/api/PushTokenApi.ktcore/network/src/main/java/kr/co/call/network/di/NetworkModule.ktcore/network/src/main/java/kr/co/call/network/dto/push/PushTokenDto.ktgradle/libs.versions.toml
Comment on lines
+12
to
+95
| object PushNotificationExtras { | ||
| const val PUSH_TYPE = "push_type" | ||
| const val CHAT_ROOM_ID = "chatRoomId" | ||
| const val CALL_ID = "callId" | ||
| const val CHARACTER_ID = "characterId" | ||
| } | ||
|
|
||
| /** | ||
| * CHAT / NOTICE / CALL 시스템 배너 (채널 [PushChannels.GENERAL]) | ||
| */ | ||
| object PushNotificationHelper { | ||
|
|
||
| private const val NOTICE_NOTIFICATION_ID = 1001 | ||
|
|
||
| fun showChat( | ||
| context: Context, | ||
| title: String, | ||
| body: String, | ||
| chatRoomId: Long, | ||
| ) { | ||
| notify( | ||
| context = context, | ||
| notificationId = chatNotificationId(chatRoomId), | ||
| title = title.ifBlank { "새 메시지" }, | ||
| body = body, | ||
| priority = NotificationCompat.PRIORITY_HIGH, | ||
| ) | ||
| } | ||
|
|
||
| fun showNotice( | ||
| context: Context, | ||
| title: String, | ||
| body: String, | ||
| ) { | ||
| notify( | ||
| context = context, | ||
| notificationId = NOTICE_NOTIFICATION_ID, | ||
| title = title.ifBlank { "알림" }, | ||
| body = body, | ||
| priority = NotificationCompat.PRIORITY_HIGH, | ||
| ) | ||
| } | ||
|
|
||
| fun showCall( | ||
| context: Context, | ||
| characterName: String, | ||
| callId: Long, | ||
| ) { | ||
| notify( | ||
| context = context, | ||
| notificationId = callNotificationId(callId), | ||
| title = characterName.ifBlank { "전화" }, | ||
| body = "수신 전화가 왔습니다", | ||
| priority = NotificationCompat.PRIORITY_MAX, | ||
| category = NotificationCompat.CATEGORY_CALL, | ||
| ) | ||
| } | ||
|
|
||
| private fun notify( | ||
| context: Context, | ||
| notificationId: Int, | ||
| title: String, | ||
| body: String, | ||
| priority: Int, | ||
| category: String? = null, | ||
| ) { | ||
| val builder = NotificationCompat.Builder(context, PushChannels.GENERAL) | ||
| .setSmallIcon(kr.co.call.designsystem.R.drawable.ic_chat_call) | ||
| .setContentTitle(title) | ||
| .setContentText(body) | ||
| .setStyle(NotificationCompat.BigTextStyle().bigText(body)) | ||
| .setAutoCancel(true) | ||
| .setPriority(priority) | ||
|
|
||
| if (category != null) { | ||
| builder.setCategory(category) | ||
| } | ||
|
|
||
| try { | ||
| NotificationManagerCompat.from(context).notify(notificationId, builder.build()) | ||
| } catch (e: SecurityException) { | ||
| Timber.w(e, "알림 표시 실패 (권한 확인)") | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
알림에 클릭 동작(PendingIntent)이 없습니다.
PushNotificationExtras는 CHAT_ROOM_ID, CALL_ID, CHARACTER_ID 키를 정의합니다. 하지만 notify()(Line 70-95)는 이 키들을 어디에도 사용하지 않습니다. setContentIntent를 호출하지 않으므로, 사용자가 알림을 탭해도 아무 화면도 열리지 않습니다.
PushPayload.kt의 문서 주석은 CHAT 알림 클릭 시 채팅방으로 이동해야 한다고 명시합니다. CALL 알림은 수락/거절/채팅 이동 UI를 제공해야 한다고 명시합니다. 현재 구현은 두 요구사항을 모두 충족하지 못합니다.
showChat에는 chatRoomId를 담은 콘텐츠 인텐트를 추가하십시오. showCall에는 최소한 채팅방 이동을 위한 콘텐츠 인텐트를 추가하고, 수락/거절 액션은 addAction으로 추가하는 것을 검토하십시오.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/kr/co/call/callfromai/push/PushNotificationHelper.kt`
around lines 12 - 95, 알림에 탭 동작이 연결되지 않았습니다. PushNotificationHelper의 showChat과
showCall에서 각각 chatRoomId와 callId/characterName을 포함한 명시적 PendingIntent를 생성해
notify 호출에 전달하고, notify의 NotificationCompat.Builder에 setContentIntent를 설정하십시오.
채팅 알림은 해당 채팅방으로 이동하게 하고, 통화 알림은 최소한 채팅방 이동을 제공하며 기존 PushNotificationExtras 키를
재사용해 수신 화면이 필요한 데이터를 받을 수 있도록 하십시오.
Comment on lines
+50
to
+60
| suspend fun unregisterCurrentDevice() { | ||
| val token = fcmTokenDataStore.getToken() | ||
| ?: fcmTokenProvider.getToken() | ||
|
|
||
| if (!token.isNullOrBlank()) { | ||
| pushTokenRepository.delete(token) | ||
| .onSuccess { Timber.d("FCM 토큰 서버 삭제 성공") } | ||
| .onFailure { error -> Timber.w(error, "FCM 토큰 서버 삭제 실패") } | ||
| } | ||
|
|
||
| fcmTokenDataStore.clearToken() |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)PushTokenManager\.kt$|CallFromAiFirebaseMessagingService|MyPageRepositoryImpl|pushTokenRepository|FcmToken' || true
echo "== PushTokenManager outline =="
if [ -f core/data/src/main/java/kr/co/call/data/push/PushTokenManager.kt ]; then
ast-grep outline core/data/src/main/java/kr/co/call/data/push/PushTokenManager.kt --view expanded || true
echo "== PushTokenManager =="
cat -n core/data/src/main/java/kr/co/call/data/push/PushTokenManager.kt
fi
echo "== firebase messaging service files =="
fd -i 'FirebaseMessagingService' .
fd -i 'CallFromAiFirebaseMessagingService' .
for f in $(fd -i 'CallFromAiFirebaseMessagingService' .); do
echo "== $f outline =="
ast-grep outline "$f" --view expanded || true
echo "== $f =="
cat -n "$f"
done
echo "== repository logout usages =="
for f in $(fd -i 'MyPageRepositoryImpl' .); do
echo "== $f outline =="
ast-grep outline "$f" --view expanded || true
echo "== $f logout sections =="
rg -n -C 8 'logout|deleteToken|unregister|deleteTokenFromServer|fcmToken' "$f" || true
done
echo "== token provider/datastore definitions =="
fd -i 'FcmTokenDataStore|FcmTokenProvider|PushTokenRepository' .
for f in $(fd -i 'FcmTokenDataStore|FcmTokenProvider|PushTokenRepository' .); do
echo "== $f =="
cat -n "$f"
doneRepository: call-from-ai/CallFromAi_Android
Length of output: 15448
FCM 삭제 실패 시 로컬 세션 정보를 유지하세요.
현재 unregisterCurrentDevice()는 실패 여부와 상관없이 fcmTokenDataStore.clearToken()을 호출하고, 이후 MyPageRepositoryImpl.logout()은 tokenDataStore.clearTokens()로 JWT와 FCM 토큰을 모두 제거합니다. 이 경우 토큰 삭제 실패 후 재시도에 사용할 계정 인증 정보가 없으므로, 삭제 재시도/취소 정책이나 토큰 해제 상태를 별도 플래그로 유지하도록 수정하세요.
또한 CallFromAiFirebaseMessagingService.onNewToken()은 로그인 상태만 확인한 뒤 별도 런타임 스코프에서 pushTokenManager.registerCurrentDevice()를 호출하므로, 로그아웃 DELETE 직후 새 토큰 등록이 완료되면 서버에 다시 연결됩니다. 토큰 삭제/등록과 onNewToken을 같은 세션 수명 주기로 직렬화하거나 로그아웃 중/후 재등록 요청을 막도록 설계해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/data/src/main/java/kr/co/call/data/push/PushTokenManager.kt` around
lines 50 - 60, Update unregisterCurrentDevice so local FCM/session data is not
cleared when repository deletion fails; preserve the authentication state or
persist an explicit pending-deletion state for retry/cancellation, and clear
tokens only after successful deletion. Coordinate MyPageRepositoryImpl.logout
with this outcome so JWT credentials remain available for the required retry
policy. Serialize deletion and registration within the same session lifecycle,
or make CallFromAiFirebaseMessagingService.onNewToken block
registerCurrentDevice during and after logout to prevent a post-logout
re-registration.
Comment on lines
+50
to
+54
| suspend fun saveToken(token: String) { | ||
| require(token.isNotBlank()) { "FCM 토큰은 비어 있을 수 없습니다." } | ||
| dataStore.edit { preferences -> | ||
| preferences[FCM_TOKEN] = token | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate candidate files =="
fd -a 'FcmTokenDataStore.kt|datastore|DataStore|Protobuf' . | sed 's#^\./##' | head -200
echo
echo "== inspect FcmTokenDataStore.kt =="
if [ -f core/datastore/src/main/java/kr/co/call/datastore/FcmTokenDataStore.kt ]; then
wc -l core/datastore/src/main/java/kr/co/call/datastore/FcmTokenDataStore.kt
cat -n core/datastore/src/main/java/kr/co/call/datastore/FcmTokenDataStore.kt
fi
echo
echo "== search datastore construction/config/encryption references =="
rg -n "DataStore|dataStore|proto|preferences|FCM_TOKEN|fcm_token|EncryptedSharedPreferences|SecretBox|Cipher|encryption|KeyProvider|provideDataStore" -S core || trueRepository: call-from-ai/CallFromAi_Android
Length of output: 12987
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== build files with app/datastore dependencies =="
fd -a 'build.gradle.kts|libs.versions.toml|gradle.properties' . | sed 's#^\./##' | while read -r f; do
if rg -n "datastore|androidx.datastore|preferences|security-crypto|EncryptedSharedPreferences|SecretBox|Hilt|dagger" "$f" >/dev/null 2>&1; then
echo "--- $f"
sed -n '1,220p' "$f"
fi
done
echo
echo "== repository-wide encryption/API terms =="
rg -n "EncryptedSharedPreferences|SecretBox|SecretBoxKeyProvider|androidx-security|security-crypto|Cipher|encrypt|decrypt|aes|AES|KeyProvider|credential|credentials|token-encryption|fcm_token" -S . || trueRepository: call-from-ai/CallFromAi_Android
Length of output: 18151
FCM 토큰을 암호화된 저장소에 저장하세요.
DataStore<Preferences>는 저장값을 암호화하지 않습니다. 현재 모듈 의존성에도 암호화 저장소 의존성이 없어 saveToken(token)이 fcm_token 원문을 저장합니다. SecretBox처럼 암호화 계층을 쓰고, 기존 fcm_token 값은 밀리거나 암호화 마이그레이션 처리하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/datastore/src/main/java/kr/co/call/datastore/FcmTokenDataStore.kt`
around lines 50 - 54, Update saveToken so the FCM token is encrypted through the
project’s established SecretBox or equivalent encryption layer before writing to
FCM_TOKEN, and add the required module dependency/imports. Ensure existing
plaintext fcm_token values are migrated to encrypted form when read or during
initialization, preserving token retrieval behavior.
Sources: Coding guidelines, Path instructions
Comment on lines
+14
to
+22
| /** | ||
| * 채팅 알림. 클릭 시 [chatRoomId] 방으로 이동한다. | ||
| * title/body는 OS notification 또는 data에서 올 수 있다. | ||
| */ | ||
| data class Chat( | ||
| val chatRoomId: Long, | ||
| val title: String, | ||
| val body: String, | ||
| ) : PushPayload |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
주석과 실제 파서 동작이 일치하지 않습니다.
Line 16 주석은 title/body가 data에서도 올 수 있다고 설명합니다. 하지만 PushPayloadParser.parseChat(다른 파일)은 notificationTitle/notificationBody 매개변수만 사용합니다. data 맵에서 title/body를 읽는 로직이 없습니다. data-only CHAT 메시지가 오면 title/body는 항상 빈 문자열이 됩니다.
PushDataKeys에 TITLE/BODY 키를 추가하고 파서에서 이를 사용하십시오. 또는 주석을 실제 동작에 맞게 수정하십시오.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/domain/src/main/java/kr/co/call/domain/model/push/PushPayload.kt` around
lines 14 - 22, Update PushPayloadParser.parseChat to read title and body from
the data map using new PushDataKeys.TITLE and PushDataKeys.BODY constants, while
preserving notificationTitle/notificationBody as fallbacks when data values are
absent; ensure data-only CHAT messages populate Chat.title and Chat.body
correctly.
Gemini AI 코드리뷰안녕하세요! 시니어 Android 개발자로서 GitHub PR을 꼼꼼히 리뷰해 드리겠습니다. 이번 PR은 Firebase Cloud Messaging(FCM) 통합에 초점을 맞추고 있네요. 전반적으로 FCM 통합이 잘 이루어졌고, 여러 모범 사례를 따르고 있어 매우 인상적입니다. 특히 PR 요약이 PR은 앱에 Firebase Cloud Messaging (FCM) 기능을 통합합니다. 주요 변경 사항은 다음과 같습니다:
전반적인 피드백FCM 통합 작업을 매우 깔끔하고 체계적으로 진행하셨습니다. 도메인, 데이터, 네트워크 레이어를 명확하게 분리하고, Hilt를 통한 의존성 주입, DataStore를 이용한 토큰 캐싱 등 모범적인 Android 아키텍처 패턴을 잘 적용하셨습니다. 특히 FCM 토큰의 등록/해제 로직을 로그인/로그아웃 플로우에 통합한 점은 칭찬할 만합니다. 세부 코드 리뷰1. Kotlin 코드 리뷰
2. Jetpack Compose 코드 확인
3. Repository/DataSource 레이어 확인
4. ViewModel 확인
5. FCM/SSE/실시간 통신 관련 코드 확인
개선 및 제안 사항
결론전반적으로 매우 완성도 높은 PR입니다. FCM 통합의 모든 주요 측면을 잘 다루었고, 아키텍처 원칙을 충실히 따랐습니다. 위에서 제시된 몇 가지 개선 및 제안 사항들을 반영한다면 더욱 완벽한 코드가 될 것입니다. 승인 전에 위 제안 사항들을 검토해 주시면 감사하겠습니다. 수고하셨습니다! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/kr/co/call/callfromai/AppViewModel.kt (1)
23-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift푸시 등록 정책을
core:domainUseCase로 위임하세요.
AppViewModel이TokenDataStore에서 인증 상태를 읽고PushTokenManager를 직접 호출합니다. 인증 확인과 토큰 등록은 비즈니스 정책입니다.LoginRepositoryImpl.ktLine 33-60에서도 같은 등록을 직접 호출하므로 정책이 여러 계층에 분산됩니다.core:domainUseCase가 등록 정책을 소유하고 ViewModel은 해당 UseCase만 호출하도록 정리하세요.As per path instructions:
**/*ViewModel.kt의 비즈니스 로직은 UseCase(core:domain) 호출로 위임해야 합니다.Also applies to: 46-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/kr/co/call/callfromai/AppViewModel.kt` around lines 23 - 25, AppViewModel의 인증 상태 확인과 PushTokenManager 직접 호출을 제거하고, 푸시 등록 정책을 담당하는 core:domain UseCase에 위임하세요. 생성자 의존성을 해당 UseCase로 교체하고, ViewModel의 관련 로직이 UseCase만 호출하도록 수정하세요. LoginRepositoryImpl의 중복 등록 호출도 동일한 UseCase를 재사용하도록 정리해 정책이 한 곳에만 존재하게 하세요.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/kr/co/call/callfromai/notification/NotificationPermission.kt`:
- Around line 37-41: Update requestNotificationPermissionIfNeeded() so
markRequested(this) is not called before launching the permission request;
persist KEY_HAS_REQUESTED from the launcher result callback after the request
completes, regardless of the granted value, while preserving the existing
permission-check behavior.
---
Nitpick comments:
In `@app/src/main/java/kr/co/call/callfromai/AppViewModel.kt`:
- Around line 23-25: AppViewModel의 인증 상태 확인과 PushTokenManager 직접 호출을 제거하고, 푸시 등록
정책을 담당하는 core:domain UseCase에 위임하세요. 생성자 의존성을 해당 UseCase로 교체하고, ViewModel의 관련
로직이 UseCase만 호출하도록 수정하세요. LoginRepositoryImpl의 중복 등록 호출도 동일한 UseCase를 재사용하도록 정리해
정책이 한 곳에만 존재하게 하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6cb11844-f139-4cf3-9c0f-a45e0992af22
📒 Files selected for processing (11)
app/src/main/java/kr/co/call/callfromai/AppViewModel.ktapp/src/main/java/kr/co/call/callfromai/MainActivity.ktapp/src/main/java/kr/co/call/callfromai/notification/NotificationPermission.ktapp/src/main/java/kr/co/call/callfromai/push/CallFromAiFirebaseMessagingService.ktapp/src/main/java/kr/co/call/callfromai/push/FirebaseFcmTokenProvider.ktapp/src/main/java/kr/co/call/callfromai/push/PushNotificationHelper.ktcore/data/src/main/java/kr/co/call/data/di/RepositoryModule.ktcore/data/src/main/java/kr/co/call/data/push/PushTokenManager.ktcore/domain/src/main/java/kr/co/call/domain/model/push/PushPayload.ktcore/domain/src/main/java/kr/co/call/domain/model/push/PushPayloadParser.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (1)
- core/data/src/main/java/kr/co/call/data/di/RepositoryModule.kt
🚧 Files skipped from review as they are similar to previous changes (7)
- gradle/libs.versions.toml
- app/src/main/java/kr/co/call/callfromai/MainActivity.kt
- app/src/main/java/kr/co/call/callfromai/push/FirebaseFcmTokenProvider.kt
- core/domain/src/main/java/kr/co/call/domain/model/push/PushPayload.kt
- core/domain/src/main/java/kr/co/call/domain/model/push/PushPayloadParser.kt
- app/src/main/java/kr/co/call/callfromai/push/CallFromAiFirebaseMessagingService.kt
- app/src/main/java/kr/co/call/callfromai/push/PushNotificationHelper.kt
Comment on lines
+37
to
+41
| fun markRequested(context: Context) { | ||
| context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) | ||
| .edit() | ||
| .putBoolean(KEY_HAS_REQUESTED, true) | ||
| .apply() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'NotificationPermission\.(shouldAutoRequest|markRequested)|notificationPermissionLauncher\.(launch|unregister)|registerForActivityResult' \
--glob '*.kt' || trueRepository: call-from-ai/CallFromAi_Android
Length of output: 169
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Kotlin files:"
git ls-files '*.kt' | sed -n '1,120p'
echo
echo "Find Notification-related files:"
fd -i 'notification|permission|MainActivity' . -e kt || true
echo
echo "Search relevant identifiers broadly:"
rg -n -i 'notificationpermission|notification permission|shouldautorequest|markrequested|permissionlauncher|registerforactivityresult' \
--glob '*.kt' . || trueRepository: call-from-ai/CallFromAi_Android
Length of output: 11827
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "NotificationPermission.kt:"
cat -n app/src/main/java/kr/co/call/callfromai/notification/NotificationPermission.kt
echo
echo "MainActivity.kt top section:"
cat -n app/src/main/java/kr/co/call/callfromai/MainActivity.kt | sed -n '1,80p'Repository: call-from-ai/CallFromAi_Android
Length of output: 4301
요청 이력을 실행 가능한 상태가 되면 저장하세요.
requestNotificationPermissionIfNeeded()는 launch(...) 실행 전에 markRequested(this)를 호출합니다. 콜백이 실행되더라도 granted 여부와 상관없이 KEY_HAS_REQUESTED가 저장되므로, 현재 흐름은 요청 실패/결과 전달 실패 시 재요청이 영구 차단됩니다. 런처 결과 콜백에서 이력을 저장하거나, 실행 전에는 저장하지 않도록 조정하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/kr/co/call/callfromai/notification/NotificationPermission.kt`
around lines 37 - 41, Update requestNotificationPermissionIfNeeded() so
markRequested(this) is not called before launching the permission request;
persist KEY_HAS_REQUESTED from the launcher result callback after the request
completes, regardless of the granted value, while preserving the existing
permission-check behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📄 작업 내용 요약
FCM 푸시 알림 환경 설정하고 토큰 발급을 구현합니다.
📎 Issue 번호
✅ 작업 목록
📝 기타 참고사항
Summary by CodeRabbit