Skip to content

Conversation

@david-parkk
Copy link
Contributor

@david-parkk david-parkk commented Jan 2, 2026

#️⃣ 연관된 이슈

closes #이슈번호 입력해주세요.

📝작업 내용

  • 메세지 예외 처리

작업 상세 내용

FCM 토큰을 보유하지 않은 사용자의 경우, FCM Push 처리 중지

💬리뷰 요구사항

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요.

Summary by CodeRabbit

출시 노트

  • 버그 수정
    • 푸시 알림 전송 시 토큰 없는 사용자에 대한 처리 개선 및 전송 신뢰성 향상

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 2, 2026

Walkthrough

FcmService에서 단일 사용자 토큰 조회 방식을 배치 처리로 변경했습니다. 모든 사용자 ID의 토큰을 일괄 조회한 후 토큰 보유 여부로 필터링하고, 토큰이 있는 사용자들만 기존 sendToUsers 메서드로 위임하는 방식으로 개선되었습니다.

Changes

코호트 / 파일 변경 사항 요약
FCM 배치 토큰 처리
src/main/java/ku_rum/backend/domain/alarm/application/FcmService.java
단일 사용자 토큰 조회에서 배치 조회로 변경 / 토큰 보유/미보유 사용자 ID 분리 처리 / 토큰 없는 사용자 조기 반환 처리 / 필터링된 수신자 목록으로 sendToUsers 위임 / 기존 공개 메서드 시그니처는 유지

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • [Fix] 메세지 예외 처리 #379: FcmService에서 토큰 누락 시 FCM 발송 방지—PR #379는 단일 사용자 토큰 확인을 위해 sendToUsersIfTokenExists 메서드를 추가한 반면, 본 PR은 배치 필터링으로 구현합니다.
  • [Feat] Notification 추가 #362: FcmService.java에서 sendToUsers 흐름 수정—PR #362는 Notification 페이로드를 sendToUsers에 추가하는 반면, 본 PR은 수신자 토큰 배치 처리를 변경합니다.
  • [Fix] FCM Gradle + Function #361: FcmService 구현 변경—PR #361에서 Firebase SDK 호출을 업데이트한 것처럼, 본 PR도 토큰 선택 및 배치 처리 방식을 개선합니다.

Poem

🐰 한 명씩 찾던 토큰, 이제 모두 함께
배치로 불러 필터링하고
없는 것들은 빨리 돌려보내
있는 것들만 남겨 전송하니
효율이 춤을 춘다 ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed 제목은 풀 리퀘스트의 주요 변경 사항인 FCM 메시지 예외 처리를 명확하게 요약하고 있습니다.
Description check ✅ Passed 템플릿의 모든 필수 섹션이 포함되어 있으며, 작업 내용과 상세 내용이 적절히 작성되어 있습니다.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/main/java/ku_rum/backend/domain/alarm/application/FcmService.java (2)

40-41: 데이터베이스 중복 조회를 제거하세요.

현재 구현에서는 사용자와 토큰 데이터를 이 메서드에서 조회한 후(40-41번 줄), 필터링된 DTO로 sendToUsers를 호출할 때 동일한 데이터를 다시 조회합니다(75, 77번 줄). 이는 불필요한 데이터베이스 부하를 발생시킵니다.

이미 조회한 토큰 데이터를 sendToUsers에 전달하거나, 필터링된 토큰으로 직접 메시지를 전송하는 방식을 고려하세요.

🔎 중복 조회를 제거하는 리팩토링 제안

방안 1: 토큰을 직접 사용하여 메시지 전송 (권장)

     if (userIdsWithToken.isEmpty()) {
         log.info(
                 "FCM 전송 가능한 유저가 없습니다. 요청 userIds={}",
                 fcmDirectDto.userIds()
         );
         return;
     }

-    FcmDirectDto filteredDto = FcmDirectDto.builder()
-            .title(fcmDirectDto.title())
-            .body(fcmDirectDto.body())
-            .userIds(userIdsWithToken)
-            .build();
-    sendToUsers(filteredDto);
+    List<String> tokens = userFcmTokens.stream()
+            .map(UserFcmToken::getToken)
+            .toList();
+    
+    MulticastMessage message = MulticastMessage.builder()
+            .addAllTokens(tokens)
+            .setNotification(
+                    Notification.builder()
+                            .setTitle(fcmDirectDto.title())
+                            .setBody(fcmDirectDto.body())
+                            .build()
+            )
+            .putData("title", fcmDirectDto.title())
+            .putData("body", fcmDirectDto.body())
+            .build();
+
+    try {
+        firebaseMessaging.sendEachForMulticast(message);
+    } catch (FirebaseMessagingException e) {
+        log.error("FCM error code: {}", e.getErrorCode());
+        log.error("FCM message: {}", e.getMessage(), e);
+        e.printStackTrace();
+        throw new GlobalException(FCM_SEND_ERROR);
+    }
 }

방안 2: sendToUsers 메서드를 오버로드하여 이미 조회한 데이터를 전달

sendToUsers 메서드에 토큰 리스트를 받는 오버로드 버전을 추가하세요.

Also applies to: 66-71


43-49: Set을 사용하여 필터링 성능을 개선하세요.

48번 줄에서 userIdsWithToken.contains(userId)를 사용하여 리스트에서 멤버십을 확인하고 있습니다. 이는 O(n*m) 시간 복잡도를 가지며, 사용자 수가 많을 경우 성능 저하를 일으킬 수 있습니다.

userIdsWithTokenSet으로 변환하면 O(1) 조회가 가능하여 전체 필터링 시간 복잡도를 O(n+m)으로 개선할 수 있습니다.

🔎 Set을 사용한 성능 개선 제안
-    List<Long> userIdsWithToken = userFcmTokens.stream()
+    Set<Long> userIdsWithToken = userFcmTokens.stream()
             .map(token -> token.getUser().getId())
-            .toList();
+            .collect(Collectors.toSet());

     List<Long> userIdsWithoutToken = fcmDirectDto.userIds().stream()
             .filter(userId -> !userIdsWithToken.contains(userId))
             .toList();

파일 상단에 import 추가:

+import java.util.Set;
+import java.util.stream.Collectors;

66번 줄에서 FcmDirectDto를 생성할 때는 List로 변환:

     FcmDirectDto filteredDto = FcmDirectDto.builder()
             .title(fcmDirectDto.title())
             .body(fcmDirectDto.body())
-            .userIds(userIdsWithToken)
+            .userIds(new ArrayList<>(userIdsWithToken))
             .build();
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between badc48a and a27a9d9.

📒 Files selected for processing (1)
  • src/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 (2)
src/main/java/ku_rum/backend/domain/alarm/application/FcmService.java (2)

51-56: 좋은 로깅 추가입니다!

토큰이 없는 사용자를 정보 로그로 기록하는 것은 디버깅과 모니터링에 유용합니다. 로그 레벨과 메시지 내용이 적절합니다.


58-64: 조기 반환 처리가 적절합니다.

전송 가능한 사용자가 없을 때 조기 반환하여 불필요한 처리를 방지하고, 이를 로그로 기록하는 것은 좋은 방어적 프로그래밍입니다.

@github-actions
Copy link

github-actions bot commented Jan 2, 2026

Test Results

 38 files   38 suites   11s ⏱️
156 tests 156 ✅ 0 💤 0 ❌
157 runs  157 ✅ 0 💤 0 ❌

Results for commit a27a9d9.

Copy link
Contributor

@kmw10693 kmw10693 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

@kmw10693 kmw10693 merged commit 485920b into develop Jan 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants