-
Notifications
You must be signed in to change notification settings - Fork 1
feat: FCM 메시징 구현 완료 #35
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
Summary of ChangesHello @slg1119, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 Firebase Cloud Messaging (FCM)을 활용하여 백엔드에 푸시 알림 기능을 전면적으로 도입합니다. 주요 변경 사항은 FCM 설정, 회원별 FCM 토큰 관리, 친구의 다음 연락 예정일 및 기념일에 맞춰 알림을 발송하는 스케줄러 구현, 그리고 알림 테스트 및 강제 전송을 위한 API 엔드포인트 추가입니다. 이를 통해 사용자에게 시기적절한 알림을 제공하여 서비스 참여도를 높이고, 친구 관계 관리를 더욱 효과적으로 지원할 수 있게 됩니다. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
FCM 메시징 기능 구현을 위한 전반적인 변경 사항을 확인했습니다. 기능 구현 자체는 잘 이루어졌으나, 몇 가지 중요한 개선점이 필요해 보입니다. 특히, 관리자용 API의 접근 제어 누락은 심각한 보안 문제이므로 반드시 수정이 필요합니다. 또한, 스케줄러에서 발생하는 N+1 쿼리 문제와 FCM 초기화 시 리소스 누수 등 성능 및 안정성과 관련된 부분을 중점적으로 검토하고 수정을 제안했습니다. 이 외에도 FCM 전송 실패 처리, APNS 배지 설정, 코드 가독성 향상 등에 대한 의견을 포함했습니다. 추가된 테스트 코드는 다양한 시나리오를 잘 پوشش하고 있어 기능의 안정성을 높이는 데 큰 도움이 될 것입니다.
| * 특정 사용자에게 강제로 FCM 알림을 전송하는 엔드포인트 (관리자용). | ||
| * memberId를 직접 지정하여 해당 사용자에게 푸시 알림을 전송합니다. | ||
| */ | ||
| @PostMapping("/force/send") |
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.
/force/send 엔드포인트는 특정 사용자에게 강제로 알림을 보낼 수 있는 강력한 기능이지만, 현재 아무런 인증/인가 제어가 없어 누구나 이 API를 호출할 수 있는 심각한 보안 취약점이 존재합니다. 관리자만 이 기능을 사용할 수 있도록 접근을 제한해야 합니다. 예를 들어, Spring Security의 메서드 시큐리티를 활성화(@EnableMethodSecurity)하고 해당 메서드에 @PreAuthorize("hasRole('ADMIN')") 어노테이션을 추가하는 방법을 사용할 수 있습니다.
@org.springframework.security.access.prepost.PreAuthorize("hasRole('ADMIN')")
@PostMapping("/force/send")
build.gradle
Outdated
| implementation 'org.springframework.boot:spring-boot-configuration-processor' | ||
| implementation 'software.amazon.awssdk:s3:2.31.16' | ||
| implementation 'io.micrometer:micrometer-registry-prometheus' | ||
| implementation 'com.google.firebase:firebase-admin:9.4.2' |
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.
| InputStream serviceAccount; | ||
| String credentialsPath = fcmProperties.getCredentialsPath(); | ||
|
|
||
| // classpath 또는 파일 시스템 경로 지원 | ||
| if (credentialsPath.startsWith("classpath:")) { | ||
| String path = credentialsPath.substring("classpath:".length()); | ||
| serviceAccount = new ClassPathResource(path).getInputStream(); | ||
| } else { | ||
| serviceAccount = new FileInputStream(credentialsPath); | ||
| } | ||
|
|
||
| FirebaseOptions options = FirebaseOptions.builder() | ||
| .setCredentials(GoogleCredentials.fromStream(serviceAccount)) | ||
| .build(); | ||
|
|
||
| FirebaseApp.initializeApp(options); | ||
| log.info("Firebase application has been initialized successfully."); |
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.
FileInputStream과 ClassPathResource.getInputStream()으로 생성된 InputStream이 닫히지 않아 리소스 누수(resource leak)가 발생할 수 있습니다. try-with-resources 구문을 사용하여 스트림이 자동으로 닫히도록 하는 것이 안전합니다.
String credentialsPath = fcmProperties.getCredentialsPath();
// classpath 또는 파일 시스템 경로 지원
try (InputStream serviceAccount = credentialsPath.startsWith("classpath:")
? new ClassPathResource(credentialsPath.substring("classpath:".length())).getInputStream()
: new FileInputStream(credentialsPath)) {
FirebaseOptions options = FirebaseOptions.builder()
.setCredentials(GoogleCredentials.fromStream(serviceAccount))
.build();
FirebaseApp.initializeApp(options);
log.info("Firebase application has been initialized successfully.");
}| public void sendDailyFriendReminders() { | ||
| log.info("Starting daily friend reminder notifications..."); | ||
|
|
||
| LocalDate today = LocalDate.now(); | ||
| Set<UUID> processedFriendIds = new HashSet<>(); | ||
|
|
||
| // 1. nextContactAt이 오늘인 친구들 처리 | ||
| List<Friend> friendsToContact = friendRepository.findAllByNextContactAt(today); | ||
| log.info("Found {} friends to contact today based on nextContactAt", | ||
| friendsToContact.size()); | ||
|
|
||
| for (Friend friend : friendsToContact) { | ||
| sendReminderForFriend(friend, "연락 예정일입니다."); | ||
| processedFriendIds.add(friend.getFriendId()); | ||
| } | ||
|
|
||
| // 2. 오늘이 기념일인 친구들 처리 | ||
| int month = today.getMonthValue(); | ||
| int day = today.getDayOfMonth(); | ||
| List<FriendAnniversary> todayAnniversaries = | ||
| friendAnniversaryRepository.findAllByMonthAndDay(month, day); | ||
| log.info("Found {} anniversaries today", todayAnniversaries.size()); | ||
|
|
||
| for (FriendAnniversary anniversary : todayAnniversaries) { | ||
| UUID friendId = anniversary.getFriendId(); | ||
|
|
||
| // 이미 연락 예정일로 알림을 보낸 친구는 스킵 | ||
| if (processedFriendIds.contains(friendId)) { | ||
| continue; | ||
| } | ||
|
|
||
| friendRepository.findById(friendId).ifPresent(friend -> { | ||
| String reason = "오늘은 " + anniversary.getTitle() + "입니다."; | ||
| sendReminderForFriend(friend, reason); | ||
| processedFriendIds.add(friendId); | ||
| }); | ||
| } | ||
|
|
||
| log.info("Daily friend reminder notifications completed. Total notifications sent: {}", | ||
| processedFriendIds.size()); | ||
| } |
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.
sendDailyFriendReminders 메서드 내에서 루프를 돌며 memberRepository.findById()와 friendRepository.findById()를 호출하고 있어 N+1 쿼리 문제가 발생합니다. 알림 대상이 많아질 경우 심각한 성능 저하를 유발할 수 있습니다.
개선 방안:
friendsToContact와todayAnniversaries에서 필요한 모든memberId와friendId를 수집합니다.findAllById와 같은 메서드를 사용하여 관련된Member와Friend객체들을 한 번의 쿼리로 모두 조회합니다.- 조회한 객체들을
Map에 담아두고, 루프 내에서는 DB 조회 없이Map에서 가져와 사용합니다.
이렇게 수정하면 DB 호출 횟수를 크게 줄여 성능을 개선할 수 있습니다.
| } catch (FirebaseMessagingException e) { | ||
| log.error("Failed to send FCM notification to token: {}", request.getFcmToken(), e); | ||
| } |
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.
| String[] activeProfiles = environment.getActiveProfiles(); | ||
| for (String profile : activeProfiles) { | ||
| if ("test".equals(profile)) { | ||
| log.info("Test profile detected. Skipping FCM initialization."); | ||
| return; | ||
| } | ||
| } |
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.
| @Query(""" | ||
| SELECT fa | ||
| FROM FriendAnniversary fa | ||
| WHERE MONTH(fa.date) = :month AND DAY(fa.date) = :day | ||
| """) | ||
| List<FriendAnniversary> findAllByMonthAndDay(@Param("month") int month, @Param("day") int day); |
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.
| ApnsConfig apnsConfig = ApnsConfig.builder() | ||
| .setAps(Aps.builder() | ||
| .setSound("default") | ||
| .setBadge(1) |
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.
b095379 to
2b23cf1
Compare
2b23cf1 to
1bfab61
Compare
작업 내용 ⚒️
리뷰어 참고 사항 🤔
DDL