-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: 멀티디바이스에서 유효하지 않은 FID/FCM 토큰 정리 로직 추가 #130
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
Conversation
WalkthroughFCM 알림 전송을 단일 토큰에서 멀티캐스트 API로 리팩토링하고, 성공/실패/무효 토큰을 추적하는 FcmSendResult를 도입하며, FCM 실패 후 무효 토큰을 가진 디바이스를 제거하는 정리 로직을 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant NS as NotificationService
participant FS as FcmService
participant FM as FirebaseMessaging
participant DDS as DeviceDomainService
participant DR as DeviceRepository
rect rgb(220, 240, 255)
Note over NS,DR: 멀티캐스트 알림 발송 및 무효 토큰 정리 흐름
end
NS->>FS: sendMulticastNotification(tokens, title, body)
rect rgb(245, 245, 220)
FS->>FS: buildNotification(title, body)
FS->>FS: 모든 토큰에 대해 배치 메시지 생성
end
FS->>FM: sendEach(messages)
FM-->>FS: BatchResponse (successCount, failureCount, responses)
rect rgb(240, 250, 240)
FS->>FS: processFcmResponse()
FS->>FS: 무효 토큰 추출<br/>(NotRegistered, InvalidArgument)
end
FS-->>NS: FcmSendResult(successCount, failureCount, invalidTokens)
rect rgb(255, 240, 245)
alt invalidTokens 존재
NS->>NS: 로깅 (무효 토큰 경고)
NS->>DDS: removeDevicesByTokens(invalidTokens)
DDS->>DR: deleteByTokens(invalidTokens)
DR->>DR: 데이터베이스에서 토큰 삭제
end
end
NS-->>NS: 반환 (성공 여부, 발송 디바이스 수)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
batch/src/main/kotlin/org/yapp/batch/service/FcmSendResult.kt(1 hunks)batch/src/main/kotlin/org/yapp/batch/service/FcmService.kt(1 hunks)batch/src/main/kotlin/org/yapp/batch/service/NotificationService.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/device/DeviceDomainService.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/device/DeviceRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/device/DeviceJpaRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/device/DeviceRepositoryImpl.kt(1 hunks)
⏰ 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-validation
infra/src/main/kotlin/org/yapp/infra/device/DeviceJpaRepository.kt
Outdated
Show resolved
Hide resolved
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
batch/src/main/kotlin/org/yapp/batch/dto/FcmSendResult.kt(1 hunks)batch/src/main/kotlin/org/yapp/batch/service/FcmService.kt(1 hunks)batch/src/main/kotlin/org/yapp/batch/service/NotificationService.kt(4 hunks)infra/src/main/kotlin/org/yapp/infra/device/DeviceJpaRepository.kt(1 hunks)
⏰ 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-validation
| if (errorCode == MessagingErrorCode.UNREGISTERED || errorCode == MessagingErrorCode.INVALID_ARGUMENT) { | ||
| invalidTokens.add(failedToken) | ||
| logger.warn("Invalid FCM token found: {}. Error: {}", failedToken, errorCode) | ||
| } else { | ||
| logger.error("Failed to send to token: {}. Error: {}", failedToken, errorCode, sendResponse.exception) | ||
| } |
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.
INVALID_ARGUMENT 토큰 삭제 조건 재검토 필요
MessagingErrorCode.INVALID_ARGUMENT는 토큰 포맷 오류뿐 아니라 메시지 페이로드나 TTL 등 요청 파라미터 전체에 대한 유효성 실패에서도 반환됩니다. 따라서 일시적인 메시지 구성 오류만 발생해도 모든 토큰이 removeDevicesByTokens로 삭제되어 정상 토큰까지 손실될 수 있습니다. UNREGISTERED만 삭제 대상으로 유지하거나, INVALID_ARGUMENT의 상세 메시지가 실제 “invalid registration token”인지 확인한 뒤에만 삭제하도록 분기해 주세요.(firebase.google.com)
🤖 Prompt for AI Agents
In batch/src/main/kotlin/org/yapp/batch/service/FcmService.kt around lines 51 to
56, the code currently treats MessagingErrorCode.INVALID_ARGUMENT the same as
UNREGISTERED and deletes tokens; change this so only UNREGISTERED tokens are
unconditionally removed, and for INVALID_ARGUMENT inspect the exception detail
(e.g., exception.message or error description) and only add the token to
invalidTokens if the message explicitly indicates an invalid registration token
(contains "invalid registration token" or equivalent); otherwise log the
INVALID_ARGUMENT as an error/warning and do not delete the token.
|


✨ PR 제목
feat: FCM 전송 실패 시 무효 토큰 정리 로직 추가
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
FcmSendResult데이터 클래스 추가FcmService에서 단일 토큰 발송 → 멀티캐스트 전송(sendEach)으로 변경FirebaseMessagingException처리 및 무효 토큰(UNREGISTERED,INVALID_ARGUMENT) 필터링 로직 추가NotificationService에서 FCM 전송 결과 기반으로invalidTokens탐지 시deviceDomainService.removeDevicesByTokens()호출DeviceDomainService,DeviceRepository,DeviceJpaRepository에deleteByTokens/deleteByFcmTokenIn메서드 추가🧪 테스트 내역
UNREGISTERED,INVALID_ARGUMENT토큰이 감지될 경우 DB에서 삭제되는지 확인🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
💬 추가 설명 or 리뷰 포인트
MessagingErrorCode.UNREGISTERED,INVALID_ARGUMENT)을 기준으로 무효 토큰을 삭제하도록 설계했습니다.Summary by CodeRabbit
새로운 기능
개선 사항