Conversation
- 1시간마다 DB 확인 - RecruitmentScheduler.java -> RecruitmentStateChecker 이름 변경
Due-IT
added
🐞 Bug
Walkthrough이 변경은 기존의 Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as Spring Scheduler
participant RecruitmentStateChecker
participant ClubRepository
participant Club
Scheduler->>RecruitmentStateChecker: 매 시간 performTask() 호출
RecruitmentStateChecker->>ClubRepository: findAll()으로 전체 Club 조회
loop 각 Club에 대해
RecruitmentStateChecker->>Club: 모집 시작/종료일, 상태 조회
alt 모집 상태가 ALWAYS
RecruitmentStateChecker-->>Club: 상태 변경 없음
else 모집 기간 비교
RecruitmentStateChecker->>Club: 상태(UPCOMING/OPEN/CLOSED) 갱신
RecruitmentStateChecker->>ClubRepository: save(Club)
end
end
Possibly related PRs
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Results28 tests 28 ✅ 1s ⏱️ Results for commit 94fcf7f. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java (1)
1-125: 테스트 클래스 전체 구조 우수
RecruitmentStateChecker의 다양한 상태 전환 시나리오를 잘 검증하는 테스트 클래스입니다. 각 시나리오에 대한 테스트 메서드가 명확하게 분리되어 있고, 한국어로 명명된 테스트 메서드는 테스트 의도를 명확히 전달합니다.하지만 PR 설명에서 언급된
performTask()메서드의 많은 if 문과 깊은 중첩에 관한 개선 방안은 이 테스트 코드에서 확인할 수 없습니다.RecruitmentStateChecker클래스 자체를 볼 수 있다면 이에 대한 개선 제안을 할 수 있을 것입니다.backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java (2)
21-21: 주석과 실제 실행 주기가 일치하지 않습니다.주석에는 "5분마다 실행"이라고 되어 있지만, 실제 설정된 값은 60 * 60 * 1000 (1시간)입니다. 주석을 실제 실행 주기에 맞게 수정해주세요.
- @Scheduled(fixedRate = 60 * 60 * 1000) // 5분마다 실행 + @Scheduled(fixedRate = 60 * 60 * 1000) // 1시간마다 실행
22-50: 복잡한 중첩 조건문 리팩토링 제안PR 설명에서 언급하셨듯이 메서드에 중첩 조건문이 많습니다. 메서드를 더 작은 단위로 분리하여 가독성을 높이는 것이 좋겠습니다.
다음과 같이 리팩토링을 제안합니다:
@Scheduled(fixedRate = 60 * 60 * 1000) // 1시간마다 실행 @Transactional public void performTask() { List<Club> clubs = clubRepository.findAll(); ZonedDateTime now = ZonedDateTime.now(ZoneId.of("Asia/Seoul")); for (Club club : clubs) { updateClubRecruitmentStatus(club, now); } } private void updateClubRecruitmentStatus(Club club, ZonedDateTime now) { ClubRecruitmentInformation recruitInfo = club.getClubRecruitmentInformation(); // ALWAYS 상태인 경우 처리하지 않음 if (recruitInfo.getClubRecruitmentStatus() == ClubRecruitmentStatus.ALWAYS) { return; } ClubRecruitmentStatus newStatus = determineRecruitmentStatus(recruitInfo, now); club.updateRecruitmentStatus(newStatus); clubRepository.save(club); } private ClubRecruitmentStatus determineRecruitmentStatus(ClubRecruitmentInformation recruitInfo, ZonedDateTime now) { ZonedDateTime startDate = recruitInfo.getRecruitmentStart(); ZonedDateTime endDate = recruitInfo.getRecruitmentEnd(); // 시작일 또는 종료일이 없는 경우 if (startDate == null || endDate == null) { return ClubRecruitmentStatus.CLOSED; } // 현재 모집 중인 경우 if (now.isAfter(startDate) && now.isBefore(endDate)) { return ClubRecruitmentStatus.OPEN; } // 모집 종료된 경우 if (now.isAfter(endDate)) { return ClubRecruitmentStatus.CLOSED; } // 모집 시작 전인 경우 long daysUntilStart = ChronoUnit.DAYS.between(now, startDate); if (daysUntilStart <= 14) { return ClubRecruitmentStatus.UPCOMING; } return ClubRecruitmentStatus.CLOSED; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/src/main/java/moadong/MoadongApplication.java(1 hunks)backend/src/main/java/moadong/club/entity/Club.java(2 hunks)backend/src/main/java/moadong/club/entity/ClubRecruitmentInformation.java(1 hunks)backend/src/main/java/moadong/club/service/ClubProfileService.java(3 hunks)backend/src/main/java/moadong/club/service/RecruitmentScheduler.java(0 hunks)backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java(1 hunks)backend/src/main/java/moadong/global/config/SchedulerConfig.java(0 hunks)backend/src/test/java/moadong/club/service/RecruitmentSchedulerTest.java(0 hunks)backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java(1 hunks)
💤 Files with no reviewable changes (3)
- backend/src/main/java/moadong/global/config/SchedulerConfig.java
- backend/src/main/java/moadong/club/service/RecruitmentScheduler.java
- backend/src/test/java/moadong/club/service/RecruitmentSchedulerTest.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/src/main/java/moadong/club/service/ClubProfileService.java (1)
backend/src/main/java/moadong/global/util/ObjectIdConverter.java (1)
ObjectIdConverter(7-17)
🔇 Additional comments (15)
backend/src/main/java/moadong/MoadongApplication.java (1)
7-7: 스케줄링 기능 활성화 적용 완료
@EnableScheduling어노테이션을 추가하여 Spring의 스케줄링 기능을 활성화했습니다. 이는 새로운RecruitmentStateChecker컴포넌트의 주기적인 실행을 지원하는 올바른 접근법입니다.Also applies to: 11-11
backend/src/main/java/moadong/club/entity/Club.java (1)
13-13: 상태 업데이트 메서드 추가 적합
updateRecruitmentStatus메서드가 적절히 구현되었습니다. 이 메서드를 통해RecruitmentStateChecker에서 모집 상태를 쉽게 갱신할 수 있습니다.Also applies to: 98-100
backend/src/main/java/moadong/club/entity/ClubRecruitmentInformation.java (2)
83-85: null 체크 추가로 안정성 향상recruitmentStart가 null일 때의 처리를 추가하여 NPE(NullPointerException)을 방지하는 좋은 변경입니다. ZonedDateTime 변환 전 null 체크는 필수적입니다.
91-93: null 체크 추가로 안정성 향상recruitmentEnd가 null일 때의 처리를 추가하여 NPE(NullPointerException)을 방지하는 좋은 변경입니다. ZonedDateTime 변환 전 null 체크는 필수적입니다.
backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java (5)
31-47: 테스트 케이스의 명확한 시나리오 처리ALWAYS 상태의 동아리는 업데이트를 건너뛰는 시나리오가 잘 테스트되었습니다. Mock 객체 설정과 검증이 적절하게 작성되었습니다.
49-67: UPCOMING 상태 전환 테스트 적절모집 시작 14일 이내에 UPCOMING 상태로 변경되는 시나리오가 잘 테스트되었습니다. 상태 변경과 저장 호출이 올바르게 검증되었습니다.
69-87: OPEN 상태 전환 테스트 적절모집 기간 중 OPEN 상태로 변경되는 시나리오가 잘 테스트되었습니다. 상태 변경과 저장 호출이 올바르게 검증되었습니다.
89-107: CLOSED 상태 전환 테스트 적절모집 기간 종료 후 CLOSED 상태로 변경되는 시나리오가 잘 테스트되었습니다. 상태 변경과 저장 호출이 올바르게 검증되었습니다.
109-124: null 날짜 처리 테스트 적절시작일 또는 종료일이 null인 경우 CLOSED 상태로 변경되는 시나리오가 잘 테스트되었습니다. 이는 ClubRecruitmentInformation에 추가된 null 체크와 일관성 있게 동작합니다.
backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java (1)
1-20: 클래스 설계가 적절합니다!
RecruitmentStateChecker구조가 명확하게 설계되었습니다. 스프링 컴포넌트로 등록되고 생성자 주입을 통해 리포지토리를 주입받는 방식이 Spring의 권장 패턴을 따르고 있어 좋습니다.backend/src/main/java/moadong/club/service/ClubProfileService.java (5)
8-8: 적절한 임포트 변경입니다.클래스 의존성 업데이트가 올바르게 이루어졌습니다.
26-29: 코드 포맷팅 개선이 잘 적용되었습니다.들여쓰기 수정이 일관되게 적용되어 가독성이 향상되었습니다.
42-47: RecruitmentScheduler 의존성 제거가 적절하게 이루어졌습니다.이전
RecruitmentScheduler를 사용한 스케줄링 로직이 제거되고 코드가 단순화되었습니다. 이제 클럽 정보 업데이트만 수행하고 스케줄링은RecruitmentStateChecker에서 일괄적으로 처리하는 방식으로 개선되었습니다.
49-58: 코드 포맷팅 개선이 잘 적용되었습니다.들여쓰기 수정이 일관되게 적용되어 가독성이 향상되었습니다.
60-67: 메서드 시그니처 변경 및 포맷팅 개선이 잘 적용되었습니다.
validateClubUpdateRequest메서드의 시그니처 변경과 들여쓰기 수정이 적절히 이루어졌습니다.
| public void performTask() { | ||
| List<Club> clubs = clubRepository.findAll(); | ||
| for (Club club : clubs) { | ||
| ClubRecruitmentInformation recruitInfo = club.getClubRecruitmentInformation(); | ||
| ZonedDateTime recruitmentStartDate = recruitInfo.getRecruitmentStart(); | ||
| ZonedDateTime recruitmentEndDate = recruitInfo.getRecruitmentEnd(); | ||
| if (recruitInfo.getClubRecruitmentStatus() == ClubRecruitmentStatus.ALWAYS) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
스케줄링 태스크 성능 보장을 위한 조치 필요
스케줄링 메서드에 트랜잭션 관리가 없습니다. DB 작업을 포함하는 스케줄링 태스크에서는 트랜잭션 관리가 중요합니다. 또한, 모든 클럽을 한번에 가져오는 방식은 클럽 수가 많아질 경우 성능 이슈가 발생할 수 있습니다.
@Scheduled(fixedRate = 60 * 60 * 1000) // 5분마다 실행
+ @Transactional
public void performTask() {
List<Club> clubs = clubRepository.findAll();추가적으로 페이징 처리를 고려해보세요:
public void performTask() {
int pageSize = 100;
int pageNumber = 0;
Page<Club> clubsPage;
do {
Pageable pageable = PageRequest.of(pageNumber++, pageSize);
clubsPage = clubRepository.findAll(pageable);
processClubPage(clubsPage.getContent());
} while (clubsPage.hasNext());
}
private void processClubPage(List<Club> clubs) {
for (Club club : clubs) {
// 현재 로직 구현
}
}| if (recruitmentStartDate != null && recruitmentEndDate != null) { | ||
| ZonedDateTime now = ZonedDateTime.now(ZoneId.of("Asia/Seoul")); | ||
| if (now.isBefore(recruitmentStartDate)) { | ||
| long between = ChronoUnit.DAYS.between(recruitmentStartDate, now); | ||
| if (between <= 14) { | ||
| club.updateRecruitmentStatus(ClubRecruitmentStatus.UPCOMING); | ||
| } else { | ||
| club.updateRecruitmentStatus(ClubRecruitmentStatus.CLOSED); | ||
| } | ||
| } else if (now.isAfter(recruitmentStartDate) && now.isBefore(recruitmentEndDate)) { | ||
| club.updateRecruitmentStatus(ClubRecruitmentStatus.OPEN); | ||
| } else if (now.isAfter(recruitmentEndDate)) { | ||
| club.updateRecruitmentStatus(ClubRecruitmentStatus.CLOSED); | ||
| } |
There was a problem hiding this comment.
날짜 비교 로직 버그 수정 필요
34번 라인의 날짜 차이 계산 로직에 버그가 있습니다. between(recruitmentStartDate, now)는 시작일과 현재 시간의 차이를 음수로 계산합니다. 올바른 계산을 위해서는 순서를 바꿔야 합니다.
- long between = ChronoUnit.DAYS.between(recruitmentStartDate, now);
+ long between = ChronoUnit.DAYS.between(now, recruitmentStartDate);📝 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.
| if (recruitmentStartDate != null && recruitmentEndDate != null) { | |
| ZonedDateTime now = ZonedDateTime.now(ZoneId.of("Asia/Seoul")); | |
| if (now.isBefore(recruitmentStartDate)) { | |
| long between = ChronoUnit.DAYS.between(recruitmentStartDate, now); | |
| if (between <= 14) { | |
| club.updateRecruitmentStatus(ClubRecruitmentStatus.UPCOMING); | |
| } else { | |
| club.updateRecruitmentStatus(ClubRecruitmentStatus.CLOSED); | |
| } | |
| } else if (now.isAfter(recruitmentStartDate) && now.isBefore(recruitmentEndDate)) { | |
| club.updateRecruitmentStatus(ClubRecruitmentStatus.OPEN); | |
| } else if (now.isAfter(recruitmentEndDate)) { | |
| club.updateRecruitmentStatus(ClubRecruitmentStatus.CLOSED); | |
| } | |
| if (now.isBefore(recruitmentStartDate)) { | |
| - long between = ChronoUnit.DAYS.between(recruitmentStartDate, now); | |
| + long between = ChronoUnit.DAYS.between(now, recruitmentStartDate); | |
| if (between <= 14) { | |
| club.updateRecruitmentStatus(ClubRecruitmentStatus.UPCOMING); | |
| } else { | |
| club.updateRecruitmentStatus(ClubRecruitmentStatus.CLOSED); | |
| } |
| public void performTask() { | ||
| List<Club> clubs = clubRepository.findAll(); | ||
| for (Club club : clubs) { | ||
| ClubRecruitmentInformation recruitInfo = club.getClubRecruitmentInformation(); | ||
| ZonedDateTime recruitmentStartDate = recruitInfo.getRecruitmentStart(); | ||
| ZonedDateTime recruitmentEndDate = recruitInfo.getRecruitmentEnd(); | ||
| if (recruitInfo.getClubRecruitmentStatus() == ClubRecruitmentStatus.ALWAYS) { | ||
| continue; | ||
| } | ||
| if (recruitmentStartDate != null && recruitmentEndDate != null) { | ||
| ZonedDateTime now = ZonedDateTime.now(ZoneId.of("Asia/Seoul")); | ||
| if (now.isBefore(recruitmentStartDate)) { | ||
| long between = ChronoUnit.DAYS.between(recruitmentStartDate, now); | ||
| if (between <= 14) { | ||
| club.updateRecruitmentStatus(ClubRecruitmentStatus.UPCOMING); | ||
| } else { | ||
| club.updateRecruitmentStatus(ClubRecruitmentStatus.CLOSED); | ||
| } | ||
| } else if (now.isAfter(recruitmentStartDate) && now.isBefore(recruitmentEndDate)) { | ||
| club.updateRecruitmentStatus(ClubRecruitmentStatus.OPEN); | ||
| } else if (now.isAfter(recruitmentEndDate)) { | ||
| club.updateRecruitmentStatus(ClubRecruitmentStatus.CLOSED); | ||
| } | ||
| } else { | ||
| club.updateRecruitmentStatus(ClubRecruitmentStatus.CLOSED); | ||
| } | ||
| clubRepository.save(club); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
예외 처리 추가 필요
DB 작업 중 발생할 수 있는 예외에 대한 처리가 없습니다. 스케줄링 작업은 백그라운드에서 실행되므로 예외 처리와 로깅이 특히 중요합니다.
@Scheduled(fixedRate = 60 * 60 * 1000) // 1시간마다 실행
public void performTask() {
+ try {
List<Club> clubs = clubRepository.findAll();
for (Club club : clubs) {
// 기존 로직...
}
+ } catch (Exception e) {
+ // 로깅 프레임워크 사용 (예: SLF4J)
+ log.error("Club recruitment status update failed", e);
+ // 필요한 경우 알림 시스템에 통보
+ }
}
3 participants
#️⃣연관된 이슈
🔧 변경 사항 요약
논의하고 싶은 부분
RecruitmentStateChecker.performTask()를 확인해보시면 if문이 많이 사용되고, 들여쓰기가 많은 감이 있습니다. 리팩토링 시도는 해보았으나 제가 보기엔 현재 상태가 코드 읽기에도 제일 수월한거 같은데 어떻게 생각하시나요? 좋은 방법이 있다면 제안 부탁드립니다 :)
🫡 참고사항
Summary by CodeRabbit
신규 기능
버그 수정
리팩터
테스트