-
Notifications
You must be signed in to change notification settings - Fork 4
[Fix] 메세지 예외 처리 #379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fix] 메세지 예외 처리 #379
Conversation
- 친구 생성시 올바른 알림 생성
- 토큰이 없는 경우 알림 중지
워크스루FCM 알림 송수신 로직에 토큰 존재 여부 확인 단계를 추가합니다. AlarmService의 변경사항
예상 코드 리뷰 난이도🎯 2 (Simple) | ⏱️ ~10 분 관련 가능성 있는 PR
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ku_rum/backend/domain/alarm/application/AlarmService.javasrc/main/java/ku_rum/backend/domain/alarm/application/FcmService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/ku_rum/backend/domain/alarm/application/AlarmService.java (1)
60-72: 변경 사항 확인 완료, 단 FcmService 구현 검증 필요알림 비활성화 체크 후 FCM 전송 시 토큰 존재 여부를 확인하는 새로운 메서드로 변경한 것은 적절합니다. 다만
FcmService.sendToUsersIfTokenExists구현에 심각한 로직 오류가 있어 함께 수정이 필요합니다 (FcmService.java 리뷰 참고).
| public void sendToUsersIfTokenExists(FcmDirectDto fcmDirectDto) { | ||
| User user = userService.getUser(); | ||
| Optional<UserFcmToken> token = userFcmTokenRepository.findByUser(user); | ||
|
|
||
| if (token.isEmpty()) { | ||
| log.info("FCM 토큰이 존재하지 않는 유저입니다. userId={}", user.getId()); | ||
| return; | ||
| } | ||
|
|
||
| sendToUsers(fcmDirectDto); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
치명적 결함: 잘못된 사용자의 토큰을 확인하고 있습니다
현재 구현은 수신자가 아닌 현재 인증된 사용자의 FCM 토큰을 확인합니다.
문제점:
- Line 40:
userService.getUser()는 현재 인증된 사용자(예: 친구 요청을 보낸 사용자 A)를 반환합니다 - 하지만 알림을 받아야 하는 사용자는
fcmDirectDto.userIds()에 포함된 수신자들(예: 친구 요청을 받는 사용자 B)입니다 - 따라서 잘못된 사용자의 토큰 존재 여부를 확인하여 수신자가 토큰이 없어도 전송을 시도하거나, 반대로 수신자는 토큰이 있는데 전송하지 않을 수 있습니다
예시 시나리오:
- 사용자 A(토큰 있음)가 사용자 B(토큰 없음)에게 친구 요청
- 현재 코드는 A의 토큰을 확인 → 존재함 →
sendToUsers호출 sendToUsers는 B에게 전송 시도 → B는 토큰이 없어서 예외 발생 (Line 122)- 원래 의도한 예외 방지가 작동하지 않음
🔎 수정 제안
수신자 목록에서 토큰이 있는 사용자들에게만 전송하도록 변경:
public void sendToUsersIfTokenExists(FcmDirectDto fcmDirectDto) {
- User user = userService.getUser();
- Optional<UserFcmToken> token = userFcmTokenRepository.findByUser(user);
+ List<User> users = userQueryService.getUsersByIds(fcmDirectDto.userIds());
+ List<UserFcmToken> userFcmTokens = userFcmTokenRepository.findByUserIn(users);
- if (token.isEmpty()) {
- log.info("FCM 토큰이 존재하지 않는 유저입니다. userId={}", user.getId());
+ if (userFcmTokens.isEmpty()) {
+ log.info("FCM 토큰이 존재하지 않는 유저입니다. userIds={}", fcmDirectDto.userIds());
return;
}
sendToUsers(fcmDirectDto);
}참고: 이렇게 수정하면 일부 사용자만 토큰이 없는 경우에도 Line 122의 validateUserToken에서 예외가 발생합니다. 토큰이 있는 사용자에게만 전송하려면 추가 필터링 로직이 필요합니다.
Committable suggestion skipped: line range outside the PR's diff.
Test Results 38 files 38 suites 11s ⏱️ Results for commit 1335990. |
#️⃣ 연관된 이슈
closes #377
📝작업 내용
작업 상세 내용
FCM 토큰을 보유하지 않은 사용자의 경우, FCM Push 처리 중지
💬리뷰 요구사항
Summary by CodeRabbit
버그 수정
✏️ Tip: You can customize this high-level summary in your review settings.