common: MissionCyclePolicy를 common으로 이동하여 날짜 계산 로직 통일#167
Merged
Conversation
- MissionCyclePolicy를 recommendation.service → common 패키지로 이동 (public) - toMissionDate(LocalDateTime) 메서드 추가 (임의 시각의 미션 날짜 계산) - getMissionCycleStart 내부를 toMissionDate 위임으로 리팩토링 - TeamActivityService의 중복 구현(getMissionDate, MISSION_RESET_TIME) 제거 - recommendation 서비스 3곳 import 업데이트 - RecommendationEmailService 파일 전체 들여쓰기 오염 수정 Closes #166
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
MissionCyclePolicy 패키지 이동 src/main/java/com/ryu/studyhelper/common/MissionCyclePolicy.java, src/main/java/com/ryu/studyhelper/recommendation/service/MissionCyclePolicy.java |
MissionCyclePolicy를 recommendation에서 common 패키지로 이동하여 날짜 계산 로직의 단일 지점 관리를 실현합니다. |
RecommendationEmailService 리팩토링 src/main/java/com/ryu/studyhelper/recommendation/service/RecommendationEmailService.java |
Clock, MailSender, RecommendationMailBuilder, MemberRecommendationRepository 등의 의존성을 명시적 필드로 선언하고, sendEmail() 개인 메서드를 신규 추가하여 이메일 전송 로직을 재구성했습니다. 에러 처리 및 상태 로깅을 강화했습니다. |
서비스 레이어 통합 업데이트 src/main/java/com/ryu/studyhelper/recommendation/service/RecommendationService.java, src/main/java/com/ryu/studyhelper/recommendation/service/ScheduledRecommendationService.java, src/main/java/com/ryu/studyhelper/team/service/TeamActivityService.java |
MissionCyclePolicy 임포트를 추가하고, 로컬 미션 날짜 계산 로직을 MissionCyclePolicy.toMissionDate()로 통일하여 중복 제거 및 일관성을 확보했습니다. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~23 minutes
Possibly related PRs
- feat : 수동 문제 추천 기능 #63: 동일한 미션 사이클 시간 기반 로직(
getMissionCycleStart,MISSION_RESET_TIME및 클록 기반 날짜 정규화)을 수정하는 PR과 연관되어 있습니다. - fix : 2시 스케줄러 시간대 체크 오류 수정 #65:
RecommendationService의 미션 사이클 처리를 수정하는 PR과 관련이 있으며, 이 PR이MissionCyclePolicy(06:00 리셋)를 추가하는 것과 대조됩니다.
Poem
🐰 여섯 시에 맞춰 미션 춤을 추네,
공통의 숲으로 정책을 옮기고,
이메일 꾸러미는 더욱 단단해졌네.
중복 없는 깔끔한 코드, 토끼가 축하하네! 🌙✨
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check | MemberService.getAdjustedDate() 제거가 예정된 것으로 보이나 이 PR에는 포함되지 않아 불완전합니다. | PR #166의 요구사항을 완전히 충족하려면 MemberService.getAdjustedDate() 제거 및 MissionCyclePolicy 사용으로 교체하는 변경을 추가하세요. |
|
| Docstring Coverage | Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목이 변경의 주요 목적을 명확하게 요약합니다: MissionCyclePolicy를 common으로 이동하여 날짜 계산 로직 통일. |
| Linked Issues check | ✅ Passed | 코드 변경이 #166의 모든 주요 요구사항을 충족합니다: MissionCyclePolicy를 common으로 이동, toMissionDate 메서드 추가, TeamActivityService의 중복 로직 제거. |
✏️ 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
166-refactor-common-missioncyclepolicy를-common으로-이동하여-날짜-계산-로직-통일
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.
🧪 테스트 결과145 tests 143 ✅ 3s ⏱️ Results for commit 4385527. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/main/java/com/ryu/studyhelper/recommendation/service/RecommendationEmailService.java (2)
65-69:send()메서드에서sendEmail()반환값이 무시되고 있습니다.
sendAll()은 성공/실패를 추적하여BatchResult를 반환하는 반면,send()는 실패 여부를 호출자에게 전달하지 않습니다. 수동 발송 시에도 결과를 반환하면 호출자가 실패를 인지하고 대응할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ryu/studyhelper/recommendation/service/RecommendationEmailService.java` around lines 65 - 69, The send(List<MemberRecommendation>) method currently ignores the result of sendEmail(MemberRecommendation); change send to return a BatchResult (matching sendAll()) and aggregate per-item outcomes from sendEmail for each MemberRecommendation (class RecommendationEmailService, methods send and sendEmail, type BatchResult) so callers receive success/failure info; implement accumulation logic (e.g., counts or a collection of failed MemberRecommendation IDs) and return the constructed BatchResult at the end.
23-26: 클래스 레벨@Transactional이 배치 처리에 미치는 영향을 검토하세요.
sendAll()메서드가 클래스 레벨@Transactional아래에서 실행되면, 모든 이메일 발송 및 상태 변경이 단일 트랜잭션으로 묶입니다. 이로 인해:
- 대량 발송 시 장시간 트랜잭션이 DB 커넥션을 점유합니다.
sendEmail의 catch 블록 내save()(Line 90)가 실패하면 예외가 전파되어 이전 성공 건까지 모두 롤백됩니다.- 프로세스 중간 장애 시 모든 진행 상황이 소실됩니다.
배치 처리의 경우 각 건별로 트랜잭션을 분리하거나,
sendAll()에@Transactional(propagation = NOT_SUPPORTED)을 선언하고sendEmail을 별도 트랜잭션으로 분리하는 방안을 고려해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ryu/studyhelper/recommendation/service/RecommendationEmailService.java` around lines 23 - 26, 현재 클래스 레벨 `@Transactional은` RecommendationEmailService 전체(특히 sendAll())를 하나의 긴 트랜잭션으로 묶어 배치 처리 문제를 일으키므로 수정하세요: 클래스 레벨 트랜잭션을 제거하거나 sendAll()에 `@Transactional`(propagation = Propagation.NOT_SUPPORTED)로 선언해 트랜잭션을 비활성화하고, 개별 이메일 처리 로직인 sendEmail(...) 또는 그 안의 DB 저장 호출(save(...))을 `@Transactional`(propagation = Propagation.REQUIRES_NEW) 또는 적절한 개별 트랜잭션으로 분리해 각 건이 독립적으로 커밋/롤백되도록 변경하세요; 대상 위치는 RecommendationEmailService 클래스의 sendAll()와 sendEmail(...) 메서드, 그리고 save 호출 지점입니다.src/main/java/com/ryu/studyhelper/common/MissionCyclePolicy.java (1)
12-35: 정적 유틸리티 클래스에 private 생성자 및final선언 추가를 권장합니다.모든 멤버가
static이므로 인스턴스화가 불필요한데, Java는 생성자를 명시하지 않으면 묵시적으로 public 기본 생성자를 자동으로 추가합니다. 이로 인해new MissionCyclePolicy()와 같은 불필요한 인스턴스 생성이 가능해집니다. SonarQube 규칙은 이를 방지하기 위해 private 생성자를 추가하도록 권고하며, 프로젝트에 Lombok이 이미 사용 중이므로@UtilityClass어노테이션도 대안이 될 수 있습니다.♻️ 제안: private 생성자 및 final 추가
-public class MissionCyclePolicy { +public final class MissionCyclePolicy { public static final LocalTime MISSION_RESET_TIME = LocalTime.of(6, 0); + private MissionCyclePolicy() { + throw new UnsupportedOperationException("유틸리티 클래스는 인스턴스화할 수 없습니다."); + } + /**또는 Lombok을 사용하는 경우:
+import lombok.experimental.UtilityClass; + +@UtilityClass -public class MissionCyclePolicy { +public class MissionCyclePolicy {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ryu/studyhelper/common/MissionCyclePolicy.java` around lines 12 - 35, Make MissionCyclePolicy a non-instantiable utility class by marking the class final and adding a private no-arg constructor; this prevents accidental instantiation (new MissionCyclePolicy()) while keeping existing static members MISSION_RESET_TIME, getMissionCycleStart(Clock), and toMissionDate(LocalDateTime) unchanged. Alternatively, if Lombok is acceptable in this module, replace the manual approach with `@UtilityClass` on MissionCyclePolicy to achieve the same effect. Ensure the constructor is private and empty so static methods and constants continue to function normally.src/main/java/com/ryu/studyhelper/team/service/TeamActivityService.java (1)
36-108:Clock미주입으로 인한 테스트 불가 및 시간대 불일치 위험Line 105에서
LocalDateTime.now()를 인자 없이 호출하면 JVM 시스템 기본 타임존에 의존하게 됩니다.RecommendationService와ScheduledRecommendationService는 모두private final Clock clock을 주입받아 일관성 있게 사용하는 반면,TeamActivityService만 시스템 clock을 직접 사용하고 있어 두 가지 문제가 발생합니다.
- 시간대 불일치 위험: 서버가 UTC 등 KST(+09:00)가 아닌 타임존으로 운영될 경우, 오전 6시 기준 날짜 보정 로직이 의도한 한국 시간 기준이 아닌 서버 로컬 시간 기준으로 동작합니다.
- 테스트 불가: 현재 시각을 제어할 수 없어 경계값(예: 새벽 5:59 vs 06:01) 기반의 단위 테스트 작성이 불가능합니다.
Clock을 주입받도록 수정하는 것을 권장합니다.♻️ 제안: Clock 주입 추가
`@Service` `@RequiredArgsConstructor` `@Slf4j` public class TeamActivityService { + private final Clock clock; private final TeamRepository teamRepository; private final TeamMemberRepository teamMemberRepository; private final RecommendationRepository recommendationRepository; private final MemberSolvedProblemRepository memberSolvedProblemRepository; ... private QueryPeriod calculateQueryPeriod(Integer days) { int queryDays = calculateDays(days); - LocalDate endDate = MissionCyclePolicy.toMissionDate(LocalDateTime.now()); + LocalDate endDate = MissionCyclePolicy.toMissionDate(LocalDateTime.now(clock)); LocalDate startDate = endDate.minusDays(queryDays - 1); return QueryPeriod.of(queryDays, startDate, endDate); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ryu/studyhelper/team/service/TeamActivityService.java` around lines 36 - 108, The service calls LocalDateTime.now() directly in calculateQueryPeriod (via MissionCyclePolicy.toMissionDate(LocalDateTime.now())), causing timezone/testability issues; inject a java.time.Clock into TeamActivityService (add private final Clock clock, include it in the constructor or `@Autowired`) and replace LocalDateTime.now() with LocalDateTime.now(clock) inside calculateQueryPeriod (and anywhere else using system time), so behavior is deterministic and testable with a fixed Clock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/ryu/studyhelper/recommendation/service/RecommendationEmailService.java`:
- Around line 88-93: The catch block in RecommendationEmailService (likely
inside sendAll()) calls memberRecommendationRepository.save(mr) which can throw
and escape the catch, cancelling remaining processing and (with class-level
`@Transactional`) rolling back prior successes; protect this by moving the save
into a protected persistence boundary: either wrap
memberRecommendationRepository.save(mr) in its own try-catch that logs any
persistence error (so it doesn't rethrow), or extract the save into a separate
method (e.g., persistFailedRecommendation(MemberRecommendation mr)) annotated
with `@Transactional`(propagation = REQUIRES_NEW) and call that from the catch
after mr.markEmailAsFailed(); ensure any exception from the save is caught and
logged so sendAll() can continue.
---
Nitpick comments:
In `@src/main/java/com/ryu/studyhelper/common/MissionCyclePolicy.java`:
- Around line 12-35: Make MissionCyclePolicy a non-instantiable utility class by
marking the class final and adding a private no-arg constructor; this prevents
accidental instantiation (new MissionCyclePolicy()) while keeping existing
static members MISSION_RESET_TIME, getMissionCycleStart(Clock), and
toMissionDate(LocalDateTime) unchanged. Alternatively, if Lombok is acceptable
in this module, replace the manual approach with `@UtilityClass` on
MissionCyclePolicy to achieve the same effect. Ensure the constructor is private
and empty so static methods and constants continue to function normally.
In
`@src/main/java/com/ryu/studyhelper/recommendation/service/RecommendationEmailService.java`:
- Around line 65-69: The send(List<MemberRecommendation>) method currently
ignores the result of sendEmail(MemberRecommendation); change send to return a
BatchResult (matching sendAll()) and aggregate per-item outcomes from sendEmail
for each MemberRecommendation (class RecommendationEmailService, methods send
and sendEmail, type BatchResult) so callers receive success/failure info;
implement accumulation logic (e.g., counts or a collection of failed
MemberRecommendation IDs) and return the constructed BatchResult at the end.
- Around line 23-26: 현재 클래스 레벨 `@Transactional은` RecommendationEmailService 전체(특히
sendAll())를 하나의 긴 트랜잭션으로 묶어 배치 처리 문제를 일으키므로 수정하세요: 클래스 레벨 트랜잭션을 제거하거나 sendAll()에
`@Transactional`(propagation = Propagation.NOT_SUPPORTED)로 선언해 트랜잭션을 비활성화하고, 개별
이메일 처리 로직인 sendEmail(...) 또는 그 안의 DB 저장 호출(save(...))을
`@Transactional`(propagation = Propagation.REQUIRES_NEW) 또는 적절한 개별 트랜잭션으로 분리해 각 건이
독립적으로 커밋/롤백되도록 변경하세요; 대상 위치는 RecommendationEmailService 클래스의 sendAll()와
sendEmail(...) 메서드, 그리고 save 호출 지점입니다.
In `@src/main/java/com/ryu/studyhelper/team/service/TeamActivityService.java`:
- Around line 36-108: The service calls LocalDateTime.now() directly in
calculateQueryPeriod (via
MissionCyclePolicy.toMissionDate(LocalDateTime.now())), causing
timezone/testability issues; inject a java.time.Clock into TeamActivityService
(add private final Clock clock, include it in the constructor or `@Autowired`) and
replace LocalDateTime.now() with LocalDateTime.now(clock) inside
calculateQueryPeriod (and anywhere else using system time), so behavior is
deterministic and testable with a fixed Clock.
Comment on lines
+88
to
93
| } catch (Exception e) { | ||
| mr.markEmailAsFailed(); | ||
| memberRecommendationRepository.save(mr); | ||
| log.error("회원 ID {} 이메일 발송 실패", mr.getMember().getId(), e); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
catch 블록 내 save() 호출이 실패할 경우 예외가 전파됩니다.
Line 90의 memberRecommendationRepository.save(mr)가 DB 장애 등으로 실패하면, 이 예외는 catch 블록 밖으로 전파되어 sendAll()의 나머지 처리를 중단시킵니다. 클래스 레벨 @Transactional과 결합하면 이전 성공 건까지 모두 롤백될 수 있습니다.
🛡️ catch 블록 내 save 보호 제안
} catch (Exception e) {
- mr.markEmailAsFailed();
- memberRecommendationRepository.save(mr);
- log.error("회원 ID {} 이메일 발송 실패", mr.getMember().getId(), e);
+ try {
+ mr.markEmailAsFailed();
+ memberRecommendationRepository.save(mr);
+ } catch (Exception saveEx) {
+ log.error("회원 ID {} 실패 상태 저장 중 오류", mr.getMember().getId(), saveEx);
+ }
+ log.error("회원 ID {} 이메일 발송 실패", mr.getMember().getId(), e);
return false;
}📝 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.
Suggested change
| } catch (Exception e) { | |
| mr.markEmailAsFailed(); | |
| memberRecommendationRepository.save(mr); | |
| log.error("회원 ID {} 이메일 발송 실패", mr.getMember().getId(), e); | |
| return false; | |
| } | |
| } catch (Exception e) { | |
| try { | |
| mr.markEmailAsFailed(); | |
| memberRecommendationRepository.save(mr); | |
| } catch (Exception saveEx) { | |
| log.error("회원 ID {} 실패 상태 저장 중 오류", mr.getMember().getId(), saveEx); | |
| } | |
| log.error("회원 ID {} 이메일 발송 실패", mr.getMember().getId(), e); | |
| return false; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/ryu/studyhelper/recommendation/service/RecommendationEmailService.java`
around lines 88 - 93, The catch block in RecommendationEmailService (likely
inside sendAll()) calls memberRecommendationRepository.save(mr) which can throw
and escape the catch, cancelling remaining processing and (with class-level
`@Transactional`) rolling back prior successes; protect this by moving the save
into a protected persistence boundary: either wrap
memberRecommendationRepository.save(mr) in its own try-catch that logs any
persistence error (so it doesn't rethrow), or extract the save into a separate
method (e.g., persistFailedRecommendation(MemberRecommendation mr)) annotated
with `@Transactional`(propagation = REQUIRES_NEW) and call that from the catch
after mr.markEmailAsFailed(); ensure any exception from the save is caught and
logged so sendAll() can continue.
There was a problem hiding this comment.
@ryuwldnjs, 네, 이해했습니다! 해당 내용은 추후 별도 이슈로 처리하는 것으로 확인했습니다.
필요하시다면 제가 이 내용을 추적할 수 있도록 GitHub 이슈를 생성해드릴 수 있습니다. 이슈 생성을 원하시면 말씀해주세요! 😊
🧠 Learnings used
Learnt from: CR
Repo: codemate-kr/codemate_BE PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-08T08:33:02.185Z
Learning: Applies to **/*Scheduler.java : Implement ProblemRecommendationScheduler to generate recommendations daily at 6 AM and EmailSendScheduler to send emails daily at 9 AM
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
1 participant
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.
관련 이슈
변경 내용
변경 유형
테스트
스크린샷 (UI 변경 시)
참고사항
Summary by CodeRabbit
릴리스 노트
리팩토링
버그 수정