feat: Expo 푸시 알림 발송 연동#73
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 37 minutes and 57 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 Walkthrough변경 요약Expo Push 알림 서비스와의 연동을 완성하여 서버에서 생성된 알림을 사용자 디바이스로 실제 푸시 발송하는 기능을 구현했습니다. 설정 관리, 도메인 모델, Expo API 클라이언트, 이벤트 기반 디스패처, 토큰 관리, 배달 로그 기록을 포함합니다. 변경 내용Expo Push 알림 발송 연동
시퀀스 다이어그램sequenceDiagram
participant App as React Native 앱
participant Server as NotificationService
participant EventBus as EventPublisher
participant Dispatcher as NotificationPushDispatcher
participant Client as ExpoPushNotificationClient
participant ExpoAPI as Expo Push API
participant DB as Database
App->>Server: 알림 생성 요청
Server->>DB: notification 저장
Server->>EventBus: NotificationCreatedEvent 발행
EventBus->>Dispatcher: 이벤트 수신 (트랜잭션 커밋 이후)
Dispatcher->>DB: 사용자/토큰 조회
Dispatcher->>Dispatcher: 사용자 활성화 확인
Dispatcher->>Dispatcher: 알림 수신 설정 확인
Dispatcher->>DB: 활성 EXPO 토큰 조회
Dispatcher->>Client: send(notification, token, payload)
Client->>ExpoAPI: POST /api/v2/push/send
ExpoAPI-->>Client: 응답 (status, messageId, error 등)
Client->>Dispatcher: PushSendResult (sent/failed/invalidToken)
Dispatcher->>DB: NotificationDeliveryLog 저장 (SENT/FAILED/SKIPPED)
alt invalidToken인 경우
Dispatcher->>DB: PushDeviceToken 비활성화 (enabled=false)
end
예상 코드 리뷰 노력🎯 4 (복잡도) | ⏱️ ~50분 관련 PR
제안 라벨
제안 검토자
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
🤖 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
`@src/main/java/com/gachi/be/domain/notification/service/NotificationPushDispatcher.java`:
- Around line 55-57: Don't trust event.userId() directly; in
NotificationPushDispatcher replace lookups that use event.userId() (e.g.,
userRepository.findById(event.userId()) and similar token lookups around the
73-75 area) with the Notification's owner id and/or validate the event id
matches the notification owner: fetch the user by notification.ownerId() (or
notification.owner() accessor), if missing call saveSkipped(notification, null,
"...") and if event.userId() is present but does not equal
notification.ownerId() treat as mismatch and skip/log, and similarly ensure any
push token looked up (tokenRepository/findByUserId or similar method) belongs to
that notification owner before sending.
- Around line 107-123: saveSkipped currently calls saveDeliveryLog which
unconditionally writes pushNotificationClient.providerName() into
NotificationDeliveryLog, causing logs to show the wrong provider; change
saveSkipped and saveDeliveryLog signatures to accept the actual provider string
(or derive it from the PushDeviceToken via token.getProvider()/token.getType())
and use that provider value in the NotificationDeliveryLog.builder() instead of
pushNotificationClient.providerName(); update all callers of saveDeliveryLog
(including saveSkipped) to pass the correct provider for each branch
(disabled/unsupported/provider-specific) so the recorded provider matches the
real one.
In
`@src/main/java/com/gachi/be/domain/notification/service/PushNotificationClient.java`:
- Around line 12-13: PushNotificationClient의 send(Notification, PushDeviceToken,
Map<String, Object>)에서 사용되는 payload의 컴파일 타임 타입 안전성을 개선하세요: 현재 Map<String,
Object> 대신 향후 확장성을 고려해 공통 베이스 레코드나 sealed interface(예: PushPayload,
ExpoPushPayload 등)를 도입하고 send 메서드 시그니처를 send(Notification, PushDeviceToken,
PushPayload)로 변경하여 제공자별 페이로드 구조를 명확히 하세요; 당장은 Expo 전용이라면 ExpoPayload 레코드를 만들고
구현체를 통해 점진적으로 리팩토링하는 방안을 적용하세요.
- Around line 7-14: Add JavaDoc to the PushNotificationClient interface:
document providerName() explaining the returned provider identifier format
(e.g., constant name or unique id) and its intended use; document
send(Notification, PushDeviceToken, Map<String,Object>) describing when it may
throw exceptions (e.g., network, invalid token, validation), the expected keys
and value types in the payload map (required vs optional fields and any nested
structures), and the semantics of the returned PushSendResult (success,
transient failure, permanent failure, retry hint) so implementers know how to
populate it; also reference the Notification and PushDeviceToken inputs and any
thread-safety or blocking behavior expectations.
In
`@src/main/java/com/gachi/be/global/config/external/NotificationPushProperties.java`:
- Around line 20-23: NotificationPushProperties.Expo의 accessToken 필드가 빈 문자열로
초기화되어 있어 "미설정" 상태를 명확히 표현하지 못하므로 accessToken의 기본값을 null로 변경하고, 이 필드에 의존하는 코드(예:
토큰 존재 여부를 검사하는 로직)들이 null-safe 하도록 검사(or Optional 사용)으로 업데이트하세요; 구체적으로
NotificationPushProperties.Expo 클래스의 private String accessToken 초기화를 제거하거나 null로
설정하고, getAccessToken/usage 지점에서 빈 문자열 체크 대신 null 체크 또는
Objects.nonNull/Optional.ofNullable로 처리하도록 수정하세요.
- Around line 12-16: Add validation to the timeout fields in
NotificationPushProperties: annotate connectTimeoutSeconds and
readTimeoutSeconds with a constraint such as `@Min`(1) or `@Positive` to prevent
zero/negative (or unreasonably small) values, import the corresponding
javax.validation constraint, and ensure the class is validated at binding by
adding `@Validated` on NotificationPushProperties (or enabling validation on the
ConfigurationProperties consumer) so invalid values fail fast.
In
`@src/test/java/com/gachi/be/domain/notification/service/NotificationPushDispatcherIntegrationTest.java`:
- Around line 98-106: The helper method registerToken currently returns the
first element of
pushDeviceTokenRepository.findAllByUserIdAndEnabledTrueAndDeletedAtIsNull(user.getId()).get(0),
which is fragile; change registerToken to explicitly verify the repository
result (e.g., assert or throw if size != 1) or filter/select the expected token
before returning so the test fails clearly when multiple tokens exist; update
references in the test to use registerToken(User, String) that returns the
verified PushDeviceToken and keep symbols pushDeviceTokenRepository,
registerToken, and PushDeviceToken to locate the change.
- Around line 46-57: In dispatchSendsPushAndRecordsDeliveryLog add an assertion
that the push client was invoked exactly once: after registering the token and
before/after checking delivery logs assert the PushNotificationClient invocation
count (either assert sendCount == 1 on your test helper/mock or use
Mockito.verify(pushNotificationClient, times(1))). Reference the test method
dispatchSendsPushAndRecordsDeliveryLog and the PushNotificationClient (or the
mock instance used in NotificationPushDispatcherIntegrationTest) so the test
explicitly fails if the client isn't called once.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f9bf940-fe5f-42ae-b77b-73d0fa16691f
📒 Files selected for processing (17)
.env.exampledeploy/.env.exampledeploy/docker-compose.ymldocs/env.mdsrc/main/java/com/gachi/be/domain/notification/entity/NotificationDeliveryLog.javasrc/main/java/com/gachi/be/domain/notification/repository/NotificationDeliveryLogRepository.javasrc/main/java/com/gachi/be/domain/notification/service/ExpoPushNotificationClient.javasrc/main/java/com/gachi/be/domain/notification/service/NotificationCreatedEvent.javasrc/main/java/com/gachi/be/domain/notification/service/NotificationPushDispatcher.javasrc/main/java/com/gachi/be/domain/notification/service/NotificationService.javasrc/main/java/com/gachi/be/domain/notification/service/PushNotificationClient.javasrc/main/java/com/gachi/be/domain/notification/service/PushSendResult.javasrc/main/java/com/gachi/be/global/config/external/ExternalApiConfig.javasrc/main/java/com/gachi/be/global/config/external/NotificationPushProperties.javasrc/main/resources/application.ymlsrc/main/resources/db/migration/V15__notification_delivery_provider.sqlsrc/test/java/com/gachi/be/domain/notification/service/NotificationPushDispatcherIntegrationTest.java
Comment on lines
+12
to
+13
| PushSendResult send( | ||
| Notification notification, PushDeviceToken pushDeviceToken, Map<String, Object> payload); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
payload 파라미터의 타입 안정성 개선을 고려하세요.
Map<String, Object> 타입은 유연하지만 컴파일 타임 타입 안정성이 없습니다. 제공자별로 필요한 payload 구조가 명확하다면, sealed interface나 공통 베이스 레코드를 도입하여 타입 안전성을 높이는 것을 향후 고려해볼 수 있습니다.
현재 단일 제공자(Expo)만 지원하는 상황에서는 Map으로 충분하지만, 제공자가 늘어날 경우 리팩토링을 검토하세요.
🤖 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
`@src/main/java/com/gachi/be/domain/notification/service/PushNotificationClient.java`
around lines 12 - 13, PushNotificationClient의 send(Notification,
PushDeviceToken, Map<String, Object>)에서 사용되는 payload의 컴파일 타임 타입 안전성을 개선하세요: 현재
Map<String, Object> 대신 향후 확장성을 고려해 공통 베이스 레코드나 sealed interface(예: PushPayload,
ExpoPushPayload 등)를 도입하고 send 메서드 시그니처를 send(Notification, PushDeviceToken,
PushPayload)로 변경하여 제공자별 페이로드 구조를 명확히 하세요; 당장은 Expo 전용이라면 ExpoPayload 레코드를 만들고
구현체를 통해 점진적으로 리팩토링하는 방안을 적용하세요.
Hminkyung
approved these changes
May 26, 2026
Collaborator
Hminkyung
left a comment
Hminkyung
left a comment
There was a problem hiding this comment.
확인했씁니다!!! 고생하셨씁니당~~~~~
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
2 participants
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.
📌 작업 요약
SKIPPEDdelivery log를 남기도록 처리notification_delivery_logs에SENT/FAILED로 기록하도록 구현provider컬럼 추가🌿 브랜치 정보
feat/#72-push-notification-integrationdevelop(기본)✅ 체크리스트
feat/refac/hotfix/chore/design/bugfix)feat/fix/refactor/docs/style/chore)🧪 테스트 결과
Summary by CodeRabbit
릴리스 노트
New Features
Documentation