Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
의존성 및 설정 backend/build.gradle, backend/src/main/java/moadong/global/config/MongoConfig.java |
ShedLock 라이브러리 추가 및 MongoDB 기반 분산 잠금 설정. MongoLockProvider 빈 구성. |
저장소 확장 backend/src/main/java/moadong/club/repository/ClubRepository.java |
모집 상태로 동아리를 조회하는 새로운 메서드 findAllByClubRecruitmentInformation_ClubRecruitmentStatus() 추가. |
스케줄링 및 알림 기능 backend/src/main/java/moadong/club/service/RecruitmentDdayNotifier.java, backend/src/main/java/moadong/club/util/RecruitmentDdayNotificationBuilder.java |
D-Day 카운트다운 알림을 송신하는 스케줄된 작업(RecruitmentDdayNotifier) 및 FCM 메시지를 구성하는 빌더(RecruitmentDdayNotificationBuilder) 구현. |
기존 스케줄러 수정 backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java |
스케줄 주기를 시간 단위에서 10분 고정 간격으로 변경하고 SchedulerLock 애너테이션 추가. |
Sequence Diagram(s)
sequenceDiagram
participant Scheduler as 스케줄러<br/>(08:00 매일)
participant RecruitmentDdayNotifier as RecruitmentDdayNotifier
participant ClubRepository as ClubRepository
participant Builder as RecruitmentDdayNotificationBuilder
participant FcmResolver as FcmTopicResolver
participant FCM as FCM
Scheduler->>RecruitmentDdayNotifier: sendDdayNotifications()
RecruitmentDdayNotifier->>ClubRepository: findAllByClubRecruitmentInformation_ClubRecruitmentStatus(OPEN)
ClubRepository-->>RecruitmentDdayNotifier: List<Club>
loop 각 동아리에 대해
RecruitmentDdayNotifier->>RecruitmentDdayNotifier: recruitmentEnd 확인 및<br/>daysLeft 계산
alt daysLeft ∈ {7, 3, 1}
RecruitmentDdayNotifier->>Builder: build(club, daysLeft)
Builder->>Builder: resolveBody(daysLeft)
Builder->>Builder: buildData(club)
Builder->>FcmResolver: resolve(clubId)
FcmResolver-->>Builder: topic
Builder-->>RecruitmentDdayNotifier: Message
RecruitmentDdayNotifier->>FCM: 알림 발송
FCM-->>RecruitmentDdayNotifier: 완료
end
end
RecruitmentDdayNotifier->>RecruitmentDdayNotifier: 송신 결과 로깅
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- [fix] 스케줄러는 db를 참조하여 모집현황을 갱신한다 #401: RecruitmentStateChecker의 스케줄링 동작을 수정하는 두 PR이 겹치며, 스케줄 간격 변경 및 ShedLock 통합을 다룸.
- [release] be #1080: 동일한 RecruitmentStateChecker 클래스를 수정하는 두 PR으로, 스케줄링 및 ShedLock 애너테이션 추가 관련.
- [feature] 개발서버에서 동아리 모집 상태 변경시 라이브서버 푸시알림 전송안되게 변경한다 #1079: RecruitmentDdayNotificationBuilder가 사용하는 FcmTopicResolver와 관련된 FCM 토픽 해석 기능을 다루는 PR.
Suggested reviewers
- PororoAndFriends
- Zepelown
- alsdddk
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Out of Scope Changes check | ❓ Inconclusive | RecruitmentStateChecker의 스케줄 간격 변경(시간 단위에서 10분 고정율로)은 기존 스케줄러와의 동일한 패턴 적용과 관련된 필요한 조정으로 보여지나, MOA-638 요구사항의 직접적인 범위를 약간 벗어난 변경입니다. | RecruitmentStateChecker의 스케줄 간격 변경이 현재 PR의 핵심 목표(D-Day 알림)와의 직접적인 관계를 명확히 하거나, 별도의 이슈/PR로 분리하는 것을 고려하세요. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목이 변경사항의 핵심을 명확하게 반영하고 있으며, D-Day 알림 기능 추가라는 주요 변경사항을 간결하고 구체적으로 설명하고 있습니다. |
| Linked Issues check | ✅ Passed | PR의 모든 코드 변경사항이 MOA-638 이슈의 요구사항을 충족하고 있습니다: D-7, D-3, D-1 일정에 따른 FCM 알림 스케줄링 구현, ShedLock을 통한 중복 실행 방지, OPEN 상태 필터링, MongoDB 기반 락 제공자 설정이 모두 포함되어 있습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#1187-noti-dday-clubs-MOA-638
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 help to get the list of available commands and usage tips.
Test Results72 tests 72 ✅ 16s ⏱️ Results for commit 93027f6. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@backend/build.gradle`:
- Around line 77-78: The ShedLock dependencies
('net.javacrumbs.shedlock:shedlock-spring:7.6.0' and
'net.javacrumbs.shedlock:shedlock-provider-mongo:7.6.0') may not be tested
against Spring Boot 3.3.8; either (A) verify compatibility by running
integration tests (startup, scheduled tasks, distributed locking flows) with the
current Spring Boot 3.3.8 and keep the coordinates if green, or (B) upgrade
Spring Boot to a tested baseline (>=3.4.0) and then update Gradle's Spring Boot
version so these ShedLock artifacts are officially supported; if neither is
possible, downgrade the ShedLock artifacts to the last version explicitly tested
with Spring Boot 3.3.x and update the dependency coordinates accordingly. Ensure
you update the build.gradle dependency entries mentioned above and run the test
suite after changing versions.
In `@backend/src/main/java/moadong/club/service/RecruitmentDdayNotifier.java`:
- Around line 45-59: In RecruitmentDdayNotifier’s loop over openClubs, a thrown
exception from club.sendPushNotification(...) currently aborts the whole loop;
fix this by wrapping the per-club notification logic (the block that builds the
notification and calls club.sendPushNotification) in a try-catch, catch
Exception (or a more specific exception if appropriate), log the failure
including club.getId(), club.getName() and the exception, and continue so
sentCount and processing of remaining clubs proceed; ensure successful sends
still increment sentCount and that notificationBuilder.build(club, daysLeft)
remains inside the try.
In `@backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java`:
- Around line 27-28: The `@SchedulerLock` on RecruitmentStateChecker uses
lockAtMostFor = "10s" which is too short given the task calls findAll() and
saves each club; increase the lock duration to at least "1m" (or remove the
explicit value so the scheduler uses MongoConfig's defaultLockAtMostFor = "2m")
to prevent lock expiry and concurrent runs across replicas; update the
`@SchedulerLock` annotation (or align it with MongoConfig) to use lockAtMostFor =
"1m" or higher while leaving lockAtLeastFor as needed.
In `@backend/src/main/java/moadong/global/config/MongoConfig.java`:
- Around line 21-24: The lockProvider method in MongoConfig currently hardcodes
the database name via mongoClient.getDatabase("shedLock"); change this to use a
configurable database name instead: inject the application's MongoDB database
name (e.g. via `@Value`("${spring.data.mongodb.database:yourDefault}") or by
injecting MongoProperties and calling getDatabase()) and pass that variable into
MongoClient.getDatabase(databaseName) (or, alternatively, use the same
MongoClient/Database instance already used by the application) so ShedLock uses
the application DB or a configurable DB name instead of the hardcoded
"shedLock".
🧹 Nitpick comments (2)
backend/src/main/java/moadong/club/repository/ClubRepository.java (1)
22-22: 적절한 Spring Data 네이밍 규칙 사용.중첩 프로퍼티 경로에
_구분자를 사용하는 것이 올바릅니다. 다만, 이 쿼리가 스케줄러에서 주기적으로 호출되므로clubRecruitmentInformation.clubRecruitmentStatus필드에 MongoDB 인덱스가 있는지 확인하시기 바랍니다. 인덱스가 없으면 클럽 수가 많아질 때 풀 컬렉션 스캔이 발생할 수 있습니다.backend/src/main/java/moadong/club/util/RecruitmentDdayNotificationBuilder.java (1)
32-39:resolveBody의 D-Day 값들이RecruitmentDdayNotifier.NOTIFICATION_DAYS와 암묵적으로 동기화되어야 합니다.현재
NOTIFICATION_DAYS = Set.of(7L, 3L, 1L)과resolveBody의 switch case(7, 3, 1)가 별도로 관리됩니다. 하나를 수정하면서 다른 쪽을 빠뜨리면 런타임에IllegalArgumentException이 발생합니다. D-Day 값과 메시지를Map<Long, String>등으로 한곳에서 관리하면 이런 불일치를 방지할 수 있습니다.
| for (Club club : openClubs) { | ||
| ClubRecruitmentInformation info = club.getClubRecruitmentInformation(); | ||
| ZonedDateTime recruitmentEnd = info.getRecruitmentEnd(); | ||
|
|
||
| if (recruitmentEnd == null) { | ||
| continue; | ||
| } | ||
|
|
||
| long daysLeft = ChronoUnit.DAYS.between(today, recruitmentEnd.toLocalDate()); | ||
|
|
||
| if (NOTIFICATION_DAYS.contains(daysLeft)) { | ||
| log.info("D-Day 알림 전송 - clubId: {}, clubName: {}, D-{}", club.getId(), club.getName(), daysLeft); | ||
| club.sendPushNotification(notificationBuilder.build(club, daysLeft)); | ||
| sentCount++; | ||
| } |
There was a problem hiding this comment.
개별 클럽 알림 전송 실패 시 나머지 클럽 알림이 모두 중단됩니다.
club.sendPushNotification()이 예외를 던지면 for 루프가 즉시 종료되어, 이후 클럽들은 알림을 받지 못합니다. 클럽별로 try-catch를 적용하여 한 건의 실패가 전체에 영향을 주지 않도록 해야 합니다.
수정 제안
for (Club club : openClubs) {
- ClubRecruitmentInformation info = club.getClubRecruitmentInformation();
- ZonedDateTime recruitmentEnd = info.getRecruitmentEnd();
-
- if (recruitmentEnd == null) {
- continue;
- }
-
- long daysLeft = ChronoUnit.DAYS.between(today, recruitmentEnd.toLocalDate());
-
- if (NOTIFICATION_DAYS.contains(daysLeft)) {
- log.info("D-Day 알림 전송 - clubId: {}, clubName: {}, D-{}", club.getId(), club.getName(), daysLeft);
- club.sendPushNotification(notificationBuilder.build(club, daysLeft));
- sentCount++;
+ try {
+ ClubRecruitmentInformation info = club.getClubRecruitmentInformation();
+ ZonedDateTime recruitmentEnd = info.getRecruitmentEnd();
+
+ if (recruitmentEnd == null) {
+ continue;
+ }
+
+ long daysLeft = ChronoUnit.DAYS.between(today, recruitmentEnd.withZoneSameInstant(SEOUL_ZONE).toLocalDate());
+
+ if (NOTIFICATION_DAYS.contains(daysLeft)) {
+ log.info("D-Day 알림 전송 - clubId: {}, clubName: {}, D-{}", club.getId(), club.getName(), daysLeft);
+ club.sendPushNotification(notificationBuilder.build(club, daysLeft));
+ sentCount++;
+ }
+ } catch (Exception e) {
+ log.error("D-Day 알림 전송 실패 - clubId: {}, clubName: {}", club.getId(), club.getName(), e);
}
}🤖 Prompt for AI Agents
In `@backend/src/main/java/moadong/club/service/RecruitmentDdayNotifier.java`
around lines 45 - 59, In RecruitmentDdayNotifier’s loop over openClubs, a thrown
exception from club.sendPushNotification(...) currently aborts the whole loop;
fix this by wrapping the per-club notification logic (the block that builds the
notification and calls club.sendPushNotification) in a try-catch, catch
Exception (or a more specific exception if appropriate), log the failure
including club.getId(), club.getName() and the exception, and continue so
sentCount and processing of remaining clubs proceed; ensure successful sends
still increment sentCount and that notificationBuilder.build(club, daysLeft)
remains inside the try.
There was a problem hiding this comment.
에러가 따로 예외로 나오진않을듯? club.sendNoti 함수는 내부에서 try-catch중이라서
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the Club entity and examine the sendPushNotification method implementation
ast-grep --pattern 'sendPushNotification($$$) {
$$$
}'
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Also search with rg to find the method definition
rg -n -A 20 'sendPushNotification' --type=java
Length of output: 4768
@lepitaaar 맞습니다! 확인해보니 Club.sendPushNotification() 메서드가 내부적으로 FirebaseMessagingException을 try-catch로 처리하고 있네요. 예외를 다시 던지지 않고 로깅만 하기 때문에 루프가 중단되지 않습니다.
제 리뷰 코멘트가 잘못되었습니다. 추가적인 try-catch 래핑이 필요하지 않습니다. 명확하게 설명해주셔서 감사합니다! 👍
Zepelown
left a comment
There was a problem hiding this comment.
그냥 send하면 어떤 사람한테 보낼지는 fcm이 알아서 처리하나요?
club.sendNoti의 Message에 담긴 topic에따라 해당 topic을 구독한 사용자한테자동으로 전송됩니다 |
#️⃣연관된 이슈
#1187
📝작업 내용
매일 오전 8시(KST) 모집 마감일 기준 D-7, D-3, D-1 동아리에 FCM 알림을 전송하는 스케줄러를 구현했습니다.
변경 사항
알림 메시지
중점적으로 리뷰받고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
릴리즈 노트
New Features
Chores