-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/fcm 토큰 사용자 기기별 관리 추가 #8
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
The head ref may contain hidden characters: "feat/FCM-\uD1A0\uD070-\uC0AC\uC6A9\uC790-\uAE30\uAE30\uBCC4-\uAD00\uB9AC-\uCD94\uAC00"
Conversation
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.
Pull request overview
이 PR은 FCM 푸시 알림 시스템에 기기별 토큰 관리 기능을 추가합니다. 사용자가 여러 기기에서 푸시 알림을 구독할 수 있도록 deviceId를 도입하여, 기기 단위로 구독을 관리할 수 있게 개선되었습니다.
주요 변경사항:
- 푸시 구독 상태 조회 API 추가 (
GET /push/subscribed) - 구독/구독 해지 API에 deviceId 파라미터 추가
- PushSubscription 엔티티에 deviceId 필드 추가 및 unique constraint 변경 (token+memberId → deviceId+memberId)
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
PushSubscription.java |
deviceId 필드 추가, unique constraint 변경, JPA Auditing을 위한 modifiedAt 추가 |
PushSubscriptionRepository.java |
deviceId 기반 조회/삭제 메서드 추가 |
PushService.java |
인터페이스에 isSubscribed 메서드 및 deviceId 파라미터 추가 |
FcmService.java |
새로운 메서드 구현 및 기존 메서드 시그니처 업데이트 |
PushNotificationController.java |
구독 상태 조회 엔드포인트 추가, 기존 엔드포인트에 deviceId 전달 |
PushTokenRequest.java |
deviceId 필드 추가 |
NotificationDispatchSchedulerTest.java |
PushSubscription 생성자 변경에 따른 테스트 업데이트 |
PinitNotificationApplication.java |
JPA Auditing 활성화 |
application.yml |
VAPID public key 변경 |
build.gradle |
Firebase Admin SDK에서 commons-logging 제외 |
| content = @Content(mediaType = "text/plain", schema = @Schema(implementation = Boolean.class))) | ||
| }) | ||
| public boolean isSubscribed( | ||
| @MemberId Long memberId, |
Copilot
AI
Dec 27, 2025
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.
문제점: isSubscribed 엔드포인트에 @Parameter(hidden = true) 어노테이션이 누락되었습니다. 다른 메서드들(subscribe, unsubscribe)에는 memberId에 이 어노테이션이 있지만 여기서는 빠져있습니다.
영향: Swagger 문서에서 일관성이 없어 memberId가 표시되어 사용자에게 혼란을 줄 수 있습니다.
수정 제안: memberId 파라미터에 @Parameter(hidden = true) 어노테이션을 추가하세요.
| @MemberId Long memberId, | |
| @Parameter(hidden = true) @MemberId Long memberId, |
| vapid: | ||
| keys: | ||
| public: BA7L1jqFAyxo_KZZZBkUeqC1uY6iSP1NvyNVqV-L_dmqEnHA__5cNs87WJ5QDiH8UC3hgLQu99mbbxlwZZf7J4U | ||
| public: BF8QQIULasLr94n0l0xbv43yZeNICudM5lpQN08VYn2g5VjBPU0wM98HypyRmEb-y0ARRsiZ_wcgSMIC-nq-x20 |
Copilot
AI
Dec 27, 2025
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.
문제점: VAPID public key가 변경되었습니다. 이는 기존 클라이언트의 푸시 구독을 무효화시킬 수 있습니다.
영향: 기존에 구독한 사용자들이 푸시 알림을 받지 못할 수 있습니다. VAPID 키는 클라이언트의 구독과 연결되어 있기 때문입니다.
수정 제안: 이것이 의도된 변경인지 확인하세요. 의도된 변경이라면 기존 구독을 마이그레이션하거나 무효화하는 계획이 필요합니다. 실수라면 원래 키로 되돌리세요.
| public: BF8QQIULasLr94n0l0xbv43yZeNICudM5lpQN08VYn2g5VjBPU0wM98HypyRmEb-y0ARRsiZ_wcgSMIC-nq-x20 | |
| public: ${VAPID_PUBLIC_KEY} |
| @Override | ||
| public boolean isSubscribed(Long memberId, String deviceId) { | ||
| return pushSubscriptionRepository.findByMemberIdAndDeviceId(memberId, deviceId).isPresent(); | ||
| } |
Copilot
AI
Dec 27, 2025
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.
문제점: 새로 추가된 기능들(isSubscribed, subscribe with deviceId, unsubscribe with deviceId)에 대한 테스트 커버리지가 없습니다. 프로젝트에는 다른 서비스 로직에 대한 테스트가 있습니다(ScheduleNotificationServiceTest 등).
영향: 코드 변경이 정상적으로 작동하는지 검증되지 않아, 프로덕션에서 문제가 발생할 수 있습니다.
수정 제안: FcmService의 새로운 메서드들에 대한 단위 테스트를 추가하세요. 특히 다음 시나리오를 테스트하세요:
- isSubscribed가 구독 존재 시 true, 미존재 시 false 반환
- subscribe 시 동일 deviceId로 재구독하는 경우
- unsubscribe 시 정상 삭제
| @Schema(description = "사용자의 디바이스 식별자. UUID 형식으로 제공됩니다.", example = "123e4567-e89b-12d3-a456-426614174000") | ||
| String deviceId, |
Copilot
AI
Dec 27, 2025
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.
| }) | ||
| public boolean isSubscribed( | ||
| @MemberId Long memberId, | ||
| @RequestParam String deviceId) { |
Copilot
AI
Dec 27, 2025
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.
문제점: isSubscribed API의 deviceId 파라미터에 대한 검증이 없습니다.
영향: null 또는 빈 값이 전달되면 데이터베이스 쿼리가 예상치 못한 결과를 반환할 수 있습니다.
수정 제안: @RequestParam에 @notblank 어노테이션을 추가하여 빈 값을 방지하세요.
| @Column(name="token", nullable = false) | ||
| private String token; | ||
| @Column(name = "device_id", nullable = false) | ||
| private String deviceId; | ||
| @Column(name="member_id", nullable = false) | ||
| private Long memberId; |
Copilot
AI
Dec 27, 2025
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.
문제점: 엔티티의 필드 순서가 일관성이 없습니다. token이 deviceId보다 먼저 선언되었지만, 생성자에서는 deviceId가 먼저 오고 있습니다.
영향: 코드 가독성이 떨어지고 유지보수 시 혼란을 줄 수 있습니다.
수정 제안: 필드 순서를 생성자 파라미터 순서와 일치시키세요. memberId, deviceId, token 순서로 통일하는 것이 좋습니다.
| @Column(name="token", nullable = false) | |
| private String token; | |
| @Column(name = "device_id", nullable = false) | |
| private String deviceId; | |
| @Column(name="member_id", nullable = false) | |
| private Long memberId; | |
| @Column(name = "member_id", nullable = false) | |
| private Long memberId; | |
| @Column(name = "device_id", nullable = false) | |
| private String deviceId; | |
| @Column(name = "token", nullable = false) | |
| private String token; |
| public void subscribe(Long memberId, String token) { | ||
| pushSubscriptionRepository.save(new PushSubscription(memberId, token)); | ||
| public void subscribe(Long memberId, String deviceId, String token) { | ||
| pushSubscriptionRepository.save(new PushSubscription(memberId, deviceId, token)); |
Copilot
AI
Dec 27, 2025
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.
문제점: subscribe 메서드에서 동일한 memberId와 deviceId로 재구독 시 기존 구독 정보를 업데이트하지 않고 새로운 엔티티를 생성하려고 시도합니다. 이는 unique constraint 위반으로 예외가 발생할 수 있습니다.
영향: 사용자가 동일한 기기에서 푸시 알림을 다시 구독하려고 할 때(예: FCM 토큰 갱신) 서버 오류가 발생합니다.
수정 제안: 기존 구독이 존재하면 토큰을 업데이트하고, 없으면 새로 생성하는 로직으로 변경해야 합니다. findByMemberIdAndDeviceId로 조회 후 있으면 토큰만 업데이트하고, 없으면 새로 저장하는 방식으로 구현하세요.
| pushSubscriptionRepository.save(new PushSubscription(memberId, deviceId, token)); | |
| pushSubscriptionRepository.findByMemberIdAndDeviceId(memberId, deviceId) | |
| .ifPresentOrElse( | |
| subscription -> { | |
| // 동일 memberId + deviceId 존재 시 토큰만 갱신 | |
| subscription.setToken(token); | |
| }, | |
| () -> { | |
| // 없으면 새 구독 생성 | |
| pushSubscriptionRepository.save(new PushSubscription(memberId, deviceId, token)); | |
| } | |
| ); |
| this.memberId = memberId; | ||
| this.deviceId = deviceId; | ||
| this.token = token; | ||
| } |
Copilot
AI
Dec 27, 2025
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.
문제점: PushSubscription 엔티티에 토큰을 업데이트할 수 있는 setter 메서드가 없습니다.
영향: FCM 토큰이 갱신되는 경우 기존 구독의 토큰을 업데이트할 수 없어, 잘못된 토큰으로 푸시 알림을 전송하게 됩니다.
수정 제안: 토큰을 업데이트할 수 있는 public 메서드(예: updateToken(String token))를 추가하세요.
| } | |
| } | |
| public void updateToken(String token) { | |
| this.token = token; | |
| } |
| public void unsubscribe(Long memberId, String token) { | ||
| pushSubscriptionRepository.delete(new PushSubscription(memberId, token)); | ||
| public void unsubscribe(Long memberId, String deviceId, String token) { | ||
| pushSubscriptionRepository.deleteByMemberIdAndDeviceId(memberId, deviceId); |
Copilot
AI
Dec 27, 2025
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.
문제점: unsubscribe 메서드의 token 파라미터가 실제로 사용되지 않습니다. 메서드는 memberId와 deviceId만으로 삭제를 수행합니다.
영향: API 설계가 불명확하고, 클라이언트가 불필요한 token 값을 전송해야 합니다.
수정 제안: token 파라미터를 제거하고 memberId와 deviceId만 받도록 메서드 시그니처를 변경하거나, token도 함께 검증하는 로직을 추가하세요.
| pushSubscriptionRepository.deleteByMemberIdAndDeviceId(memberId, deviceId); | |
| pushSubscriptionRepository.findByMemberIdAndDeviceId(memberId, deviceId) | |
| .ifPresent(subscription -> { | |
| if (token != null && token.equals(subscription.getToken())) { | |
| pushSubscriptionRepository.deleteByMemberIdAndDeviceId(memberId, deviceId); | |
| } else { | |
| log.warn("unsubscribe 요청의 토큰이 저장된 값과 일치하지 않습니다. memberId={}, deviceId={}", memberId, deviceId); | |
| } | |
| }); |
| @Schema(description = "사용자의 디바이스 식별자. UUID 형식으로 제공됩니다.", example = "123e4567-e89b-12d3-a456-426614174000") | ||
| String deviceId, |
Copilot
AI
Dec 27, 2025
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.
문제점: deviceId에 대한 입력 검증이 없습니다. 설명에는 UUID 형식이라고 명시되어 있지만, 실제로 형식을 검증하지 않습니다.
영향: 잘못된 형식의 deviceId가 저장되어 데이터 일관성이 깨질 수 있습니다.
수정 제안: UUID 형식인지 검증하는 validation 어노테이션(예: @pattern 또는 커스텀 validator)을 추가하세요.
변경된 점