Conversation
- notification을 제거, title과 body를 notification이 아닌 data.title, data.body에 담기
|
Caution Review failedThe pull request is closed. Walkthrough스케줄러의 sendPreAlarmNotifications에 시작/종료 로그가 추가되고, FcmService에 데이터 전용 멀티캐스트 전송 경로와 sendBulkNotification 메서드가 도입되어 토큰 중복 제거, 본문 기준 그룹화, 플랫폼별 설정, 배치(<=500) 전송 및 결과 집계 로직이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as AlarmReminderScheduler
participant FCMService as FcmService
participant Firebase as Firebase Messaging
Scheduler->>FCMService: sendBulkNotification(targets)
activate FCMService
FCMService->>FCMService: 그룹화(by body) / 토큰 중복 제거 / 배치 생성(<=500)
loop 각 배치
FCMService->>Firebase: sendEachForMulticast(data:{title,body}, AndroidConfig, ApnsConfig)
Firebase-->>FCMService: BatchResponse(개별 SendResponse[])
FCMService->>FCMService: handleSendResult -> 성공 occurrenceId 수집, 무효 토큰 식별, member↔token 매핑
end
FCMService-->>Scheduler: FcmSendResult(성공 IDs, 무효 토큰, memberToTokens)
deactivate FCMService
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Assessment against linked issues
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Semgrep (1.128.1)src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java{"version":"1.128.1","results":[],"errors":[{"code":4,"level":"error","type":"InvalidRuleSchemaError","long_msg":"{'pattern-not-either': [{'pattern': '@GetMapping(...)\n...\n@CustomErrorCodes(...)\n...\npublic $RET $M(...){ ... }\n'}, {'pattern': '@PostMapping(...)\n...\n@CustomErrorCodes(...)\n...\npublic $RET $M(...){ ... }\n'}, {'pattern': '@PutMapping(...)\n...\n@CustomErrorCodes(...)\n...\npublic $RET $M(...){ ... }\n'}, {'pattern': '@DeleteMapping(...)\n...\n@CustomErrorCodes(...)\n...\npublic $RET $M(...){ ... }\n'}, {'pattern': '@PatchMapping(...)\n...\n@CustomErrorCodes(...)\n...\npublic $RET $M(...){ ... }\n'}]} is not valid under any of the given schemas","short_msg":"Invalid rule schema","spans":[{"file":".semgrep/semgrep.yml","start":{"line":89,"col":9,"offset":-1},"end":{"line":120,"col":5,"offset":-1},"source_hash":"b582dea1be576d791d43c1fb4f7261c55ab9e08f176bcb3c071834a52d4d2d6d"}]},{"code":7,"level":"error","type":"SemgrepError","message":"invalid configuration file found (1 configs were invalid)"}],"paths":{"scanned":[]},"skipped_rules":[]} 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (4)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java (1)
66-72: 무효 토큰 정리 로직 버그: 잘못된 memberId로 remove 호출 + 일부 토큰 누락 가능
- 현재 구현은 “성공한 멤버 목록(memberToTokens)”을 기준으로 모든 invalidTokens를 각 멤버에 대해 제거 시도합니다. 그 결과:
- 실패만 발생한 멤버의 무효 토큰은 제거되지 않습니다.
- 반대로 다른 멤버에게 속한 토큰을 불필요하게 여러 멤버에 대해 remove 시도합니다(O(N_members)×O(N_invalidTokens)).
- 정확한 매핑(토큰 → 소유 memberId)을 기준으로 제거해야 합니다. targets를 이용해 token→memberId 맵을 만들고, invalidTokens 각각에 대해 해당 소유자에게서만 제거하도록 변경하세요.
아래 diff로 교체하면 정확성과 성능 모두 개선됩니다.
- // 4. 무효 토큰 정리(멤버 매핑 정보가 있으면 거기서 제거) - for (Map.Entry<Long, List<String>> e : result.getMemberToTokens().entrySet()) { - Long memberId = e.getKey(); - for (String bad : result.getInvalidTokens()) { - redisService.removeInvalidToken(memberId, bad); - } - } + // 4. 무효 토큰 정리: 토큰 소유자 기준으로 정확히 제거 + Map<String, Long> tokenToMemberId = targets.stream() + .collect(java.util.stream.Collectors.toMap( + PushTargetDto::token, + PushTargetDto::memberId, + (a, b) -> a // 동일 토큰에 대해 첫 매핑 유지 + )); + for (String bad : result.getInvalidTokens()) { + Long owner = tokenToMemberId.get(bad); + if (owner != null) { + redisService.removeInvalidToken(owner, bad); + } + }src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (3)
139-146: 민감정보/자격증명 노출 위험: 실패 로그에 FCM 토큰 원문이 출력됩니다.FCM 토큰은 자격증명에 준하는 민감 정보입니다. 원문 대신 마스킹/해시 후 일부만 로그에 남기세요.
권장 수정:
- log.warn("FCM 실패: token={}, error={}", dto.token(), fme.getErrorCode()); + log.warn("FCM 실패: token={}, error={}", maskToken(dto.token()), fme.getErrorCode()); ... - log.warn("FCM 실패(원인 미상): token={}, ex={}", dto.token(), ex != null ? ex.getClass().getSimpleName() : "null"); + log.warn("FCM 실패(원인 미상): token={}, ex={}", maskToken(dto.token()), ex != null ? ex.getClass().getSimpleName() : "null");헬퍼 메서드(파일 내 추가):
private String maskToken(String token) { if (token == null) return "null"; int len = token.length(); int head = Math.min(8, len); int tail = Math.min(4, Math.max(0, len - head)); String prefix = token.substring(0, head); String suffix = tail > 0 ? token.substring(len - tail) : ""; return prefix + "...(len=" + len + ")..." + suffix; }
151-157: 오류코드 문자열 비교 대신 SDK enum 사용을 고려하세요.문자열 비교는 취약합니다. FirebaseMessagingException의 MessagingErrorCode(enum)를 사용하면 안전합니다.
예시:
import com.google.firebase.messaging.MessagingErrorCode; private boolean isTokenInvalid(FirebaseMessagingException e) { MessagingErrorCode code = e.getMessagingErrorCode(); return code == MessagingErrorCode.UNREGISTERED || code == MessagingErrorCode.INVALID_ARGUMENT || code == MessagingErrorCode.INVALID_REGISTRATION_TOKEN; }
33-35: TITLE 하드코딩은 재사용성을 떨어뜨립니다.향후 다른 타입의 푸시에 재사용하려면 TITLE을 파라미터화(메서드 인자/DTO 필드)하거나 구성으로 외부화하는 것이 좋습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java(2 hunks)src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
📄 CodeRabbit Inference Engine (AGENT.md)
**/*.java: 세미콜론 누락, 들여쓰기 불일치, import 순서 등의 형식 오류를 검토합니다.
사용되지 않는 변수나 주석은 제거를 제안합니다.
한 줄이 너무 길거나 중첩이 과도한 경우 구조 개선을 권장합니다.
Files:
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.javasrc/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
**/*{Controller,Service,Repository}.java
📄 CodeRabbit Inference Engine (AGENT.md)
Controller/Service는
getUser,createUser,removeUser등 동사 기반으로 명명하고, Repository는findByUser,insertUser,deleteUser처럼 명확히 구분합니다.
Files:
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
🧬 Code Graph Analysis (2)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java (1)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmOccurrenceBatchScheduler.java (2)
Scheduled(22-27)Slf4j(10-47)
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (1)
src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/PushTargetDto.java (1)
Builder(5-13)
🔇 Additional comments (2)
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (2)
74-77: PR 목표 부합: Notification 제거 + data.title/body 사용이 명확합니다.data-only로 전환하면서 title/body를 data에 넣는 방향이 명시적이고, 클라이언트 구현과의 계약만 일치하면 문제 없습니다. LGTM.
60-65: 그룹핑 기준이 body(주소 문구)입니다. 토큰 중복 전송 의도 확인 필요동일 멤버가 여러 주소로 다수 알림 대상인 경우, 하나의 토큰이 서로 다른 body 그룹에 중복 포함되어 여러 번 전송됩니다. 이것이 요구사항(주소별 별도 알림)과 일치하는지 확인 부탁드립니다. 또한 address가 null인 케이스 처리도 고려가 필요합니다.
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java
Show resolved
Hide resolved
| .setAndroidConfig( | ||
| AndroidConfig.builder() | ||
| .setPriority(Priority.HIGH) // 즉시 전송 | ||
| .build() | ||
| ) | ||
| .setApnsConfig( // IOS 설정 | ||
| ApnsConfig.builder() | ||
| .putHeader("apns-priority", "10") // 즉시 전송 | ||
| .setAps( | ||
| Aps.builder() | ||
| .setContentAvailable(true) // 백그라운드 데이터 수신 허용 | ||
| .build() | ||
| ) | ||
| .build() | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
iOS 데이터 전용 푸시: apns-push-type=background 헤더 누락, 우선순위 10은 부적절할 수 있습니다(배경 데이터는 5 권장). Android/iOS TTL도 지정하세요.
- iOS silent/data-only 알림은 iOS 13+에서 apns-push-type 헤더가 요구됩니다. background 전송은 보통 priority=5가 권장됩니다. 현재 10으로 설정되어 있어 의도(백그라운드 데이터 수신)와 상충할 수 있습니다.
- 본 알림은 “1시간 전 리마인드” 용도이므로 만료 시간(TTL/expiration)도 설정해야 지연 전송을 방지할 수 있습니다.
제안 패치:
.setAndroidConfig(
AndroidConfig.builder()
- .setPriority(Priority.HIGH) // 즉시 전송
+ .setPriority(Priority.HIGH) // 즉시 전송
+ .setTtl(90 * 60_000) // TTL 90분 (필요시 조정)
.build()
)
.setApnsConfig( // IOS 설정
ApnsConfig.builder()
- .putHeader("apns-priority", "10") // 즉시 전송
+ .putHeader("apns-priority", "5") // background 권장
+ .putHeader("apns-push-type", "background")
.setAps(
Aps.builder()
.setContentAvailable(true) // 백그라운드 데이터 수신 허용
.build()
)
+ // .putHeader("apns-expiration", String.valueOf(Instant.now().plus(90, ChronoUnit.MINUTES).getEpochSecond()))
.build()
)최신 가이던스 확인 요청:
🌐 Web query:
APNs silent/data-only remote notification requires which apns-push-type and priority? Is priority=5 recommended for background content-available notifications, and is the apns-push-type=background header mandatory on iOS 13+?
💡 Result:
Short answer
-
For a silent (data-only / content-available) APNs push:
- apns-push-type: "background". (docs.azure.cn, kodeco.com)
- apns-priority: "5" (low priority / power‑aware). (developer.apple.com, kodeco.com)
-
Yes — notifications whose payload contains only "content-available":1 should be sent with priority 5 (Apple treats 10 as immediate and “an error” for content-only background pushes). (developer.apple.com, kodeco.com)
-
Yes — the apns-push-type header is required for delivery to devices running iOS 13+ (and watchOS 6+); if it’s missing or mismatched APNs may delay, drop, or return an error. The header is an HTTP/2 request header (not a key in the JSON payload). (developer.apple.com, docs.azure.cn)
Notes / practical tips
- If your notification also contains alert/sound/badge (user‑visible content), use apns-push-type: "alert" and apns-priority: "10". (developer.apple.com)
- apns-push-type and apns-priority are HTTP/2 headers you send to APNs (not fields inside the aps JSON). (docs.azure.cn, developer.apple.com)
If you want, I can show an example HTTP/2 POST to api.push.apple.com with the exact headers + JSON payload for a silent notification.
Citations:
- 1: https://docs.azure.cn/en-us/notification-hubs/push-notification-updates-ios-13?utm_source=chatgpt.com
- 2: https://www.kodeco.com/books/push-notifications-by-tutorials/v2.0/chapters/3-remote-notification-payload?utm_source=chatgpt.com
- 3: https://developer.apple.com/library/archive/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/CommunicatingwithAPNs.html?utm_source=chatgpt.com
- 4: https://www.kodeco.com/books/push-notifications-by-tutorials/v2.0/chapters/3-remote-notification-payload?utm_source=chatgpt.com
- 5: https://developer.apple.com/library/archive/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/CommunicatingwithAPNs.html?utm_source=chatgpt.com
- 6: https://www.kodeco.com/books/push-notifications-by-tutorials/v2.0/chapters/3-remote-notification-payload?utm_source=chatgpt.com
- 7: https://developer.apple.com/forums/thread/118420?utm_source=chatgpt.com
- 8: https://docs.azure.cn/en-us/notification-hubs/push-notification-updates-ios-13?utm_source=chatgpt.com
- 9: https://developer.apple.com/library/archive/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/CommunicatingwithAPNs.html?utm_source=chatgpt.com
- 10: https://docs.azure.cn/en-us/notification-hubs/push-notification-updates-ios-13?utm_source=chatgpt.com
- 11: https://developer.apple.com/library/archive/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/CommunicatingwithAPNs.html?utm_source=chatgpt.com
iOS 백그라운드 전용 푸시: apns-push-type/priority 수정 및 Android·iOS TTL 설정 필요
아래 항목을 반영해주세요:
- AndroidConfig에 TTL 설정 추가
- ApnsConfig에 silent 알림용 HTTP/2 헤더 추가
- apns-priority: "5" (background 권장)
- apns-push-type: "background" (iOS 13+ 필수)
- 필요 시 apns-expiration 헤더로 만료 시간 지정
File: src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
Lines: 82–96
제안 패치:
.setAndroidConfig(
AndroidConfig.builder()
- .setPriority(Priority.HIGH) // 즉시 전송
+ .setPriority(Priority.HIGH) // 즉시 전송
+ .setTtl(90 * 60_000) // TTL 90분 (필요 시 조정)
.build()
)
.setApnsConfig(
ApnsConfig.builder()
- .putHeader("apns-priority", "10") // 즉시 전송
+ .putHeader("apns-priority", "5") // background 권장
+ .putHeader("apns-push-type", "background") // iOS 13+ 필수
.setAps(
Aps.builder()
.setContentAvailable(true) // 백그라운드 데이터 수신 허용
.build()
)
+ // .putHeader("apns-expiration", String.valueOf(
+ // Instant.now().plus(90, ChronoUnit.MINUTES).getEpochSecond()
+ // ))
.build()
)참고:
- content-available 전용 푸시는 HTTP/2 헤더(
apns-push-type,apns-priority)로만 제어하며, JSON payload 내부 필드가 아닙니다. - TTL 또는 apns-expiration 설정을 통해 메시지 만료 시점을 명시적으로 관리하세요.
📝 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.
| .setAndroidConfig( | |
| AndroidConfig.builder() | |
| .setPriority(Priority.HIGH) // 즉시 전송 | |
| .build() | |
| ) | |
| .setApnsConfig( // IOS 설정 | |
| ApnsConfig.builder() | |
| .putHeader("apns-priority", "10") // 즉시 전송 | |
| .setAps( | |
| Aps.builder() | |
| .setContentAvailable(true) // 백그라운드 데이터 수신 허용 | |
| .build() | |
| ) | |
| .build() | |
| ) | |
| .setAndroidConfig( | |
| AndroidConfig.builder() | |
| .setPriority(Priority.HIGH) // 즉시 전송 | |
| .setTtl(90 * 60_000) // TTL 90분 (필요 시 조정) | |
| .build() | |
| ) | |
| .setApnsConfig( // IOS 설정 | |
| ApnsConfig.builder() | |
| .putHeader("apns-priority", "5") // background 권장 | |
| .putHeader("apns-push-type", "background") // iOS 13+ 필수 | |
| .setAps( | |
| Aps.builder() | |
| .setContentAvailable(true) // 백그라운드 데이터 수신 허용 | |
| .build() | |
| ) | |
| // .putHeader("apns-expiration", String.valueOf( | |
| // Instant.now().plus(90, ChronoUnit.MINUTES).getEpochSecond() | |
| // )) | |
| .build() | |
| ) |
🤖 Prompt for AI Agents
In src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java around
lines 82–96, the APNS headers and Android TTL are missing for silent
(background) iOS pushes and Android message expiry; add an AndroidConfig TTL
(e.g., AndroidConfig.builder().setTtl(Duration.ofSeconds(...)) alongside
existing priority), and modify the ApnsConfig to include HTTP/2 headers for
silent pushes by using putHeader("apns-push-type","background") and
putHeader("apns-priority","5") (and optionally putHeader("apns-expiration",
String.valueOf(expirationEpochSeconds)) if you need explicit expiration), while
keeping Aps.builder().setContentAvailable(true); ensure headers are added via
ApnsConfig.putHeader calls rather than message payload fields.
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (3)
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (2)
60-65: 그룹 키를 문자열 메시지가 아닌 주소로 묶고, 메시지 문자열은 루프 내에서 구성 권장현재 그룹핑 키가 최종 body 문자열입니다. 주소를 키로 사용하고 메시지 문자열은 루프에서 생성하면 메시지 템플릿 변경 시 불필요한 재그룹핑 오버헤드를 줄일 수 있습니다.
예시(diff):
- Map<String, List<PushTargetDto>> groupedByBody = targets.stream() - .collect(Collectors.groupingBy( - dto -> String.format("1시간 뒤 %s에서 알림이 울릴 예정이에요!", dto.address()) - )); + Map<String, List<PushTargetDto>> groupedByAddress = targets.stream() + .collect(Collectors.groupingBy(PushTargetDto::address)); @@ - for (Map.Entry<String, List<PushTargetDto>> entry : groupedByBody.entrySet()) { - String body = entry.getKey(); + for (Map.Entry<String, List<PushTargetDto>> entry : groupedByAddress.entrySet()) { + String body = String.format("1시간 뒤 %s에서 알림이 울릴 예정이에요!", entry.getKey()); List<PushTargetDto> group = dedupByToken(entry.getValue());
150-157: 무효 토큰 에러 코드 보강 제안환경에 따라 에러 코드 prefix가 붙는 경우가 있으니 아래 변형도 고려해 주세요.
return List.of( "registration-token-not-registered", "invalid-argument", "unregistered", - "messaging/invalid-registration-token" + "messaging/invalid-registration-token", + "messaging/registration-token-not-registered" ).contains(e.getErrorCode());src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java (1)
67-72: 무효 토큰 정리 로직 오류: 일부 토큰 미삭제 및 불필요한 N×M 반복현재 로직은 memberToTokens(“성공한 멤버”)만 순회하면서 result.invalidTokens 전체를 모든 멤버에서 제거 시도합니다. 문제:
- 성공 토큰이 없는 멤버(=모든 전송이 실패한 멤버)의 무효 토큰은 제거되지 않습니다.
- 모든 멤버에 대해 동일 토큰 제거를 반복하는 비효율(O(M×N)).
두 가지 방향 중 하나로 수정하세요.
방안 A) FcmService에서 즉시 제거(권장, 서비스 주석과 일치). 위 FcmService 코멘트의 diff 참고.
방안 B) 스케줄러에서 정확한 매핑으로 제거. targets를 이용해 token→memberId 맵을 만든 뒤 invalidTokens만 단일 루프로 제거:
- // 4. 무효 토큰 정리(멤버 매핑 정보가 있으면 거기서 제거) - for (Map.Entry<Long, List<String>> e : result.getMemberToTokens().entrySet()) { - Long memberId = e.getKey(); - for (String bad : result.getInvalidTokens()) { - redisService.removeInvalidToken(memberId, bad); - } - } + // 4. 무효 토큰 정리: 정확한 매핑으로 단일 루프 제거 + Map<String, Long> tokenToMember = targets.stream() + .collect(java.util.stream.Collectors.toMap( + PushTargetDto::token, + PushTargetDto::memberId, + (a, b) -> a // 충돌 시 최초 값 유지 + )); + for (String bad : result.getInvalidTokens()) { + Long owner = tokenToMember.get(bad); + if (owner != null) { + redisService.removeInvalidToken(owner, bad); + } + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java(2 hunks)src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
📄 CodeRabbit Inference Engine (AGENT.md)
**/*.java: 세미콜론 누락, 들여쓰기 불일치, import 순서 등의 형식 오류를 검토합니다.
사용되지 않는 변수나 주석은 제거를 제안합니다.
한 줄이 너무 길거나 중첩이 과도한 경우 구조 개선을 권장합니다.
Files:
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.javasrc/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
**/*{Controller,Service,Repository}.java
📄 CodeRabbit Inference Engine (AGENT.md)
Controller/Service는
getUser,createUser,removeUser등 동사 기반으로 명명하고, Repository는findByUser,insertUser,deleteUser처럼 명확히 구분합니다.
Files:
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
🧬 Code Graph Analysis (2)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java (1)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmOccurrenceBatchScheduler.java (2)
Slf4j(10-47)Scheduled(22-27)
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (1)
src/main/java/akuma/whiplash/infrastructure/firebase/dto/FcmSendResult.java (1)
Getter(9-15)
🔇 Additional comments (5)
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (3)
6-9: Firebase 플랫폼 설정 import 적절데이터 전용 멀티캐스트 전송을 위한 Android/APNs 설정 import 추가가 목적에 부합합니다.
72-72: 토큰 기준 중복 제거의 의미적 영향 확인 필요같은 토큰에 대해 여러 occurrenceId가 존재할 수 있는 시나리오에서 토큰 기준 dedup이 일부 occurrence의 성공 집계를 누락시킬 수 있습니다. 의도대로 “디바이스당 1회 발송”이 맞는지, 혹은 (memberId, token) 단위 dedup이 맞는지 확인 바랍니다.
159-166: 토큰 중복 제거 구현 적절LinkedHashMap 기반의 최초 등장 우선 보존으로 안정적 dedup이 이뤄집니다.
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java (2)
30-30: 시작 로그 추가 적절스케줄러 시작 지점의 라이프사이클 로그가 문제 분석에 유용합니다.
74-75: 종료 로그 추가 적절스케줄러 종료 지점의 라이프사이클 로그 추가가 유용합니다.
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
Outdated
Show resolved
Hide resolved
| .putAllData(data) | ||
| .setAndroidConfig( | ||
| AndroidConfig.builder() | ||
| .setPriority(Priority.HIGH) // 즉시 전송 | ||
| .build() | ||
| ) | ||
| .setApnsConfig( // IOS 설정 | ||
| ApnsConfig.builder() | ||
| .putHeader("apns-priority", "10") // 즉시 전송 | ||
| .setAps( | ||
| Aps.builder() | ||
| .setContentAvailable(true) // 백그라운드 데이터 수신 허용 | ||
| .build() | ||
| ) | ||
| .build() | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
iOS(APNs) 데이터 전용 푸시 설정 재검토: background 타입과 우선순위
data-only + content-available=true 케이스는 일반적으로
- apns-push-type=background
- apns-priority=5
을 권장합니다. 현재는 content-available=true와 apns-priority=10 조합으로 설정되어 있어 플랫폼 권장과 상충될 수 있습니다.
제안(diff):
- .setApnsConfig( // IOS 설정
+ .setApnsConfig( // iOS 설정: data-only background 권장 설정
ApnsConfig.builder()
- .putHeader("apns-priority", "10") // 즉시 전송
+ .putHeader("apns-push-type", "background")
+ .putHeader("apns-priority", "5")
.setAps(
Aps.builder()
.setContentAvailable(true) // 백그라운드 데이터 수신 허용
.build()
)
.build()
)추가로, 만약 “표시형(alert)” 동작을 원하시면 data-only 대신 aps.alert을 구성하고 apns-push-type=alert를 사용해야 합니다. 현재 PR 목표(데이터 전용)와 맞는지 재확인 부탁드립니다.
🏁 Script executed:
#!/bin/bash
rg -n ".setApnsConfig" -A5 -B5 src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.javaLength of output: 662
🌐 Web query:
What are the recommended APNs headers for data-only background pushes on iOS regarding apns-push-type and apns-priority?
💡 Result:
Use these APNs headers for a data‑only background push on iOS:
- apns-push-type: background[2].
- apns-priority: 5[2][5].
Essential details:
- Apple introduced apns-push-type in iOS 13; for silent/background updates it must be set to background, otherwise delivery may be delayed or dropped[2].
- For background (silent/content-available) pushes, apns-priority must be 5; priority 10 is not allowed for background-only notifications and may prevent the app from launching in the background[5]. Apple’s WWDC guidance and industry providers document this requirement; vendors note that priority 5 is required when there’s no alert payload[2].
- When sending via FCM/APNs, pair these headers with an aps payload that contains only content-available: 1 (no alert/sound/badge) to make it truly background-only[1][7].
- Example (FCM HTTP v1 headers block under apns.headers): apns-push-type: background and apns-priority: 5[1].
Citations:
- 1: https://dev.to/randomengy/data-only-fcm-push-messages-on-ios-swiftui-3pj2
- 2: https://onesignal.com/blog/ios-13-introduces-4-breaking-changes-to-notifications/
- 3: https://firebase.google.com/docs/cloud-messaging/concept-options
- 4: https://forums.kodeco.com/t/background-notification-issues/142625
- 5: https://docs.azure.cn/en-us/notification-hubs/push-notification-updates-ios-13
- 6: 🔥 iOS Background Notifications Not Being Displayed invertase/react-native-firebase#3805
- 7: https://apnspush.com/how-to-push-background-notifications
- 8: https://forum.ionicframework.com/t/push-notifications-background-handler/188578
- 9: https://docs.aws.amazon.com/sns/latest/dg/sns-send-custom-platform-specific-payloads-mobile-devices.html
- 10: https://blog.stackademic.com/ios-18-background-survival-guide-part-2-silent-push-notifications-ae44e458b109
iOS 무음 백그라운드 푸시: apns-push-type 및 우선순위 수정 필요
Apple 권장사항에 따라 data-only + content-available=true 푸시는
• apns-push-type: background
• apns-priority: 5
를 사용해야 하며, priority=10은 백그라운드 푸시에서 허용되지 않아 푸시 지연 또는 누락을 초래할 수 있습니다.
수정 대상:
- 파일: src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
- 위치:
.setApnsConfig빌더 부분 (약 87–96라인)
제안(diff):
- .setApnsConfig( // IOS 설정
+ .setApnsConfig( // iOS 무음(background) 푸시 권장 설정
ApnsConfig.builder()
- .putHeader("apns-priority", "10") // 즉시 전송
+ .putHeader("apns-push-type", "background")
+ .putHeader("apns-priority", "5")
.setAps(
Aps.builder()
.setContentAvailable(true) // 백그라운드 데이터 수신 허용
.build()
)
.build()
)또한, 알림(alert) 표시가 필요하다면 aps.alert 페이로드와 함께
apns-push-type: alert 및 apns-priority: 10 설정을 별도로 적용해야 합니다.
현재 PR 목표가 순수 데이터 전송인지 재확인 바랍니다.
📝 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.
| .putAllData(data) | |
| .setAndroidConfig( | |
| AndroidConfig.builder() | |
| .setPriority(Priority.HIGH) // 즉시 전송 | |
| .build() | |
| ) | |
| .setApnsConfig( // IOS 설정 | |
| ApnsConfig.builder() | |
| .putHeader("apns-priority", "10") // 즉시 전송 | |
| .setAps( | |
| Aps.builder() | |
| .setContentAvailable(true) // 백그라운드 데이터 수신 허용 | |
| .build() | |
| ) | |
| .build() | |
| ) | |
| .putAllData(data) | |
| .setAndroidConfig( | |
| AndroidConfig.builder() | |
| .setPriority(Priority.HIGH) // 즉시 전송 | |
| .build() | |
| ) | |
| .setApnsConfig( // iOS 무음(background) 푸시 권장 설정 | |
| ApnsConfig.builder() | |
| .putHeader("apns-push-type", "background") | |
| .putHeader("apns-priority", "5") | |
| .setAps( | |
| Aps.builder() | |
| .setContentAvailable(true) // 백그라운드 데이터 수신 허용 | |
| .build() | |
| ) | |
| .build() | |
| ) |
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
Outdated
Show resolved
Hide resolved
- 백그라운드 알림이 아닌 UI에 표시되는 푸시 알림으로 설정 - TTL은 60분
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (3)
49-51: 주석과 실제 동작 불일치: "무효 토큰은 즉시 Redis에서 제거" 명시했지만 실제로는 스케줄러에서만 제거주석(49-51줄)에서는 "무효 토큰은 즉시 Redis에서 제거"라고 명시되어 있지만, 실제로는
handleSendResult메서드에서 무효 토큰을 수집만 하고 Redis 제거는 스케줄러(AlarmReminderScheduler)에서 수행됩니다. 이는 주석과 동작이 일치하지 않습니다.다음과 같이 수정하여 서비스 내에서 즉시 제거하도록 구현하시는 것을 권장합니다:
if (fme != null) { log.warn("FCM 실패: token={}, error={}", dto.token(), fme.getErrorCode()); if (isTokenInvalid(fme)) { invalidTokens.add(dto.token()); + // 서비스 내부에서 즉시 정리 (주석과 동작 일치) + try { + redisService.removeInvalidToken(dto.memberId(), dto.token()); + } catch (Exception cleanEx) { + log.warn("무효 토큰 정리 중 오류: memberId={}, token={}", dto.memberId(), dto.token(), cleanEx); + } } } else {
76-79: 불변 Map 사용으로 코드 간결화 가능HashMap을 생성하고 put으로 값을 추가하는 대신
Map.of()를 사용하면 더 간결하고 명확합니다.- Map<String, String> data = Map.of( - "title", DEFAULT_TITLE, - "body", body - ); + Map<String, String> data = Map.of("title", DEFAULT_TITLE, "body", body);
95-95: 로그 레벨 조정 고려: 빈번한 배치 전송 시 debug 레벨 권장매 배치마다 info 레벨로 성공/실패 건수를 로깅하면 운영 환경에서 로그가 과도하게 생성될 수 있습니다.
- log.info("FCM 멀티캐스트 결과: success={}, failure={}", response.getSuccessCount(), response.getFailureCount()); + log.debug("FCM 멀티캐스트 결과: success={}, failure={}", response.getSuccessCount(), response.getFailureCount());
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java(1 hunks)src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java(8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*{Controller,Service,Repository}.java
📄 CodeRabbit Inference Engine (AGENT.md)
Controller/Service는
getUser,createUser,removeUser등 동사 기반으로 명명하고, Repository는findByUser,insertUser,deleteUser처럼 명확히 구분합니다.
Files:
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
**/*.java
📄 CodeRabbit Inference Engine (AGENT.md)
**/*.java: 세미콜론 누락, 들여쓰기 불일치, import 순서 등의 형식 오류를 검토합니다.
사용되지 않는 변수나 주석은 제거를 제안합니다.
한 줄이 너무 길거나 중첩이 과도한 경우 구조 개선을 권장합니다.
Files:
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.javasrc/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java
🧬 Code Graph Analysis (2)
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (1)
src/main/java/akuma/whiplash/infrastructure/firebase/dto/FcmSendResult.java (1)
Getter(9-15)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java (3)
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmQueryServiceImpl.java (1)
Slf4j(32-180)src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmOccurrenceEntity.java (1)
updateReminderSent(86-88)src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java (2)
Override(243-248)Slf4j(48-341)
🔇 Additional comments (4)
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (3)
146-152: LGTM! Android TTL 설정이 적절히 구현됨Duration을 활용한 TTL 설정이 깔끔하게 구현되었습니다. null 체크와 유효성 검증도 적절합니다.
154-172: iOS 알림 설정이 잘 구현됨UI 표시용 iOS 알림 설정이 적절히 구현되었습니다. apns-push-type="alert"와 priority="10" 설정이 올바르며, TTL 처리도 적절합니다.
88-90: iOS 백그라운드(데이터 전용) 푸시 설정 확인 필요현재
.setApnsConfig(buildApnsConfigAlert(...))를 사용해 UI 알림을 전송하고 있지만, 상단 주석에서는 “데이터 전용(data-only)” 멀티캐스트라고 명시되어 있어 구현 의도와 불일치합니다. 아래 중 하나를 선택해 의도를 명확히 해 주세요:
데이터 전용 푸시가 목표인 경우
- 코드 교체
- .setApnsConfig( // iOS 설정 - buildApnsConfigAlert(DEFAULT_TITLE, body, Duration.ofMinutes(60)) - ) + .setApnsConfig( // iOS 데이터 전용 설정 + buildApnsConfigBackground(Duration.ofMinutes(60)) + )buildApnsConfigBackground메서드 추가private ApnsConfig buildApnsConfigBackground(Duration ttl) { ApnsConfig.Builder apns = ApnsConfig.builder() .putHeader("apns-push-type", "background") .putHeader("apns-priority", "5") .setAps(Aps.builder() .setContentAvailable(true) .build()); if (ttl != null && !ttl.isZero() && !ttl.isNegative()) { long epochSec = (System.currentTimeMillis() + ttl.toMillis()) / 1000; apns.putHeader("apns-expiration", String.valueOf(epochSec)); } return apns.build(); }UI 알림(alert)이 목표인 경우
• 현재buildApnsConfigAlert호출이 올바르므로 상단 주석을 “UI 알림”으로 수정src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java (1)
30-77: 시작/종료 로그가 try-finally로 적절히 보장됨조기 return이 있어도 finally 블록에서 종료 로그가 항상 출력되도록 잘 구현되었습니다. 운영 모니터링에 유용한 개선입니다.
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java
Show resolved
Hide resolved
1 participant
📄 PR 요약
✍🏻 PR 상세
👀 참고사항
✅ 체크리스트
🚪 연관된 이슈 번호
Closes #51
Summary by CodeRabbit