Conversation
Contributor
WalkthroughMailLogAspect에서 finally 블록의 로컬 변수명 변경 및 실패 경로에 상세 로그 추가; MailLogRepository에 구독별로 발송된(SENT) 퀴즈 ID 집합을 반환하는 JPQL 쿼리 주석이 추가됨. 제어 흐름과 Redis 재시도 로직은 변경 없음. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant MailSender
participant MailLogAspect
participant MailLogRepo
participant Redis
Scheduler->>MailSender: sendMail(subscription, quiz)
MailSender->>MailLogAspect: around advice
alt send success
MailLogAspect->>MailLogRepo: save(MailLog(status=SENT))
else send failed
MailLogAspect->>MailLogAspect: log.info(subscriptionId, cause)
MailLogAspect->>MailLogRepo: save(MailLog(status=FAILED, caused=...))
MailLogAspect->>Redis: enqueue retry (unchanged)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
crocusia
requested review from
HeeMang-Lee,
Kimyoonbeom,
jong-0126 and
wannabeing
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
cs25-batch/src/main/java/com/example/cs25batch/aop/MailLogAspect.java (1)
46-50: MailLogAspect: 실패 로깅 레벨 및 예외 원인 보존 강화
- 현재 catch 블록에서
throw new CustomMailException(MailExceptionCode.EMAIL_SEND_FAILED_ERROR);로직으로 인해 원인 예외가 손실됩니다.
→ CustomMailException에(MailExceptionCode, Throwable)생성자 오버로드를 추가하거나, 최소한 로그에 스택트레이스를 남겨야 합니다.- 운영 가시성을 위해
log.info대신log.warn이상으로 로그 레벨을 상향하세요.조치 대상:
- cs25-batch/src/main/java/com/example/cs25batch/aop/MailLogAspect.java (46–50)
- CustomMailException 클래스
(cs25-entity/src/main/java/com/example/cs25entity/domain/mail/exception/CustomMailException.java)수정 예시:
--- a/cs25-batch/src/main/java/com/example/cs25batch/aop/MailLogAspect.java +++ b/cs25-batch/src/main/java/com/example/cs25batch/aop/MailLogAspect.java } catch (Exception e) { status = MailStatus.FAILED; - caused = e.getMessage(); - throw new CustomMailException(MailExceptionCode.EMAIL_SEND_FAILED_ERROR); + caused = e.getMessage(); + // 스택트레이스를 보존하고 WARN 레벨로 기록 + log.warn("메일 발송 실패 : subscriptionId - {}, cause - {}", subscription.getId(), caused, e); + // CustomMailException에 (MailExceptionCode, Throwable) 생성자 추가 후 사용 + throw new CustomMailException(MailExceptionCode.EMAIL_SEND_FAILED_ERROR, e); } finally { …그리고 다음과 같이 생성자를 추가하세요:
// cs25-entity/src/main/java/com/example/cs25entity/domain/mail/exception/CustomMailException.java public CustomMailException(MailExceptionCode errorCode, Throwable cause) { super(cause); this.errorCode = errorCode; this.httpStatus = errorCode.getHttpStatus(); this.message = errorCode.getMessage(); }
🧹 Nitpick comments (1)
cs25-entity/src/main/java/com/example/cs25entity/domain/mail/repository/MailLogRepository.java (1)
32-32: 메서드명 가독성 개선 제안(선택)@query를 사용하는 만큼 파스 경로 기반 네이밍(언더스코어) 규칙을 따를 필요는 없습니다. 도메인 의도가 더 명확하도록 아래와 같이 권장합니다.
-Set<Long> findDistinctQuiz_IdBySubscription_Id(@Param("subscriptionId") Long subscriptionId); +Set<Long> findDistinctQuizIdsBySubscriptionId(@Param("subscriptionId") Long subscriptionId);SENT로 한정한 경우라면 더 구체적으로:
Set<Long> findDistinctSentQuizIdsBySubscriptionId(@Param("subscriptionId") Long subscriptionId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cs25-batch/src/main/java/com/example/cs25batch/aop/MailLogAspect.java(1 hunks)cs25-entity/src/main/java/com/example/cs25entity/domain/mail/repository/MailLogRepository.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: crocusia
PR: NBC-finalProject/CS25-BE#191
File: cs25-batch/src/main/java/com/example/cs25batch/aop/MailLogAspect.java:60-60
Timestamp: 2025-06-27T09:00:50.236Z
Learning: MailLogAspect에서 mailLogRepository.flush() 호출은 배치 처리의 청크 단위 트랜잭션과 비동기 처리 환경에서 MailLog의 즉시 영속화가 필요해서 추가된 것입니다. 재시도 로직이 실패한 메일 로그에 즉시 접근해야 하는 요구사항 때문에 필수적입니다.
🔇 Additional comments (3)
cs25-entity/src/main/java/com/example/cs25entity/domain/mail/repository/MailLogRepository.java (2)
10-10: 필요한 @query 임포트 추가 적절합니다쿼리 기반 메서드 추가에 필요한 임포트가 정확합니다.
27-31: Java Text Block 지원을 위한 Java 버전 설정 확인 필요
@Query(""" … """)에 사용된 Java Text Block 문법은 JDK 15 이상에서만 지원됩니다.
현재 각 모듈의build.gradle및pom.xml에서sourceCompatibility/targetCompatibility또는 Java Toolchain 설정이 검색되지 않아, 실제 컴파일 환경의 Java 버전이 불분명합니다. 최소 JDK 15(권장 JDK 17) 이상으로 설정되어 있는지 확인해주세요.점검 및 조치 사항:
- cs25-entity를 포함한 모든 Gradle 모듈의
build.gradle에또는java { toolchain { languageVersion = JavaLanguageVersion.of(17) } }설정 추가sourceCompatibility = '17' targetCompatibility = '17'- Maven 모듈이 존재한다면
pom.xml의<maven-compiler-plugin>에또는<configuration> <release>17</release> </configuration><source>17</source>/<target>17</target>적용- 공통 설정(예: Gradle 설정 플러그인) 또는 CI 빌드 환경에서도 JDK 15+ 이상 사용 보장
cs25-batch/src/main/java/com/example/cs25batch/aop/MailLogAspect.java (1)
51-57: 변수명 정리 및 save/flush 동작 유지 👍
log→mailLog로 명확해졌습니다.save후flush유지도 적절합니다. 과거 러닝(배치/비동기 재시도 로직에서 MailLog 즉시 영속화 필요)에 부합합니다.Also applies to: 59-60
cs25-entity/src/main/java/com/example/cs25entity/domain/mail/repository/MailLogRepository.java
Outdated
Show resolved
Hide resolved
cs25-entity/src/main/java/com/example/cs25entity/domain/mail/repository/MailLogRepository.java
Outdated
Show resolved
Hide resolved
HeeMang-Lee
approved these changes
Aug 11, 2025
wannabeing
approved these changes
Aug 11, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cs25-entity/src/main/java/com/example/cs25entity/domain/mail/repository/MailLogRepository.java (2)
28-34: 선택: 재사용성 향상 — 상태 파라미터화된 오버로드 추가 제안현재 메서드는
SENT로 고정되어 있어 목적에는 정확합니다. 다만 재사용성을 위해 동일 쿼리의 상태 파라미터 버전을 오버로드로 추가하는 것을 권장합니다(기존 메서드는 유지).아래 메서드를 추가하는 방식을 고려해 주세요:
import com.example.cs25entity.domain.mail.enums.MailStatus; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; @Query(""" select distinct ml.quiz.id from MailLog ml where ml.subscription.id = :subscriptionId and ml.status = :status """) Set<Long> findDistinctQuizIdsBySubscriptionIdAndStatus( @Param("subscriptionId") Long subscriptionId, @Param("status") MailStatus status );추가로, 현 메서드의 의미가 “SENT만 반환”임을 드러내기 위해 Javadoc을 붙이거나(비파괴), 장기적으로는 메서드명을
findDistinctSentQuizIdsBySubscriptionId로 리네이밍하는 방안도 고려할 수 있습니다.
28-34: 선택: 성능 최적화 — 인덱스, 읽기 전용 힌트 고려대량 데이터 기준 최적화를 위해 다음을 검토해 주세요.
- DB 인덱스:
subscription_id,status,quiz_id복합 인덱스 추가.- 읽기 전용 힌트: Hibernate read-only 힌트를 걸어 불필요한 더티 체킹 방지.
예시(엔티티에 인덱스 추가 — 실제 컬럼명 확인 필요):
// MailLog 엔티티 @Table( name = "mail_log", indexes = { @Index(name = "idx_mail_log_subscription_status_quiz", columnList = "subscription_id,status,quiz_id") } )예시(읽기 전용 힌트):
import jakarta.persistence.QueryHint; import org.springframework.data.jpa.repository.QueryHints; @QueryHints(@QueryHint(name = "org.hibernate.readOnly", value = "true")) @Query(""" select distinct ml.quiz.id from MailLog ml where ml.subscription.id = :subscriptionId and ml.status = com.example.cs25entity.domain.mail.enums.MailStatus.SENT """) Set<Long> findDistinctQuiz_IdBySubscription_Id(@Param("subscriptionId") Long subscriptionId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cs25-entity/src/main/java/com/example/cs25entity/domain/mail/repository/MailLogRepository.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: crocusia
PR: NBC-finalProject/CS25-BE#191
File: cs25-batch/src/main/java/com/example/cs25batch/aop/MailLogAspect.java:60-60
Timestamp: 2025-06-27T09:00:50.236Z
Learning: MailLogAspect에서 mailLogRepository.flush() 호출은 배치 처리의 청크 단위 트랜잭션과 비동기 처리 환경에서 MailLog의 즉시 영속화가 필요해서 추가된 것입니다. 재시도 로직이 실패한 메일 로그에 즉시 접근해야 하는 요구사항 때문에 필수적입니다.
🔇 Additional comments (3)
cs25-entity/src/main/java/com/example/cs25entity/domain/mail/repository/MailLogRepository.java (3)
10-11: 임포트 및 @param 추가 적절 — 파라미터 바인딩 안정화명시적
@Param과 관련 임포트 추가로 런타임 바인딩 이슈 없이 안전합니다. LGTM.
28-34: SENT 상태로 필터링 + distinct 선택 — 요구사항에 부합하고 이전 피드백도 해소됨FAILED 로그가 포함되지 않도록
SENT로 제한했고,distinct로 중복 제거까지 충족합니다. 이전 리뷰에서 제기된 상태 필터 및 파라미터 바인딩 이슈가 모두 해결되었습니다.
28-34: 영향 범위 확인 완료 — 기존 호출부에서 모두 ‘SENT’ 전용으로 사용 중기존 호출부를 확인한 결과, 아래 두 곳 모두 반환값을
sentQuizIds로 명명하고size()를 통해 “전송된 문제 수”로만 처리하고 있어, FAILED 로그 제외에 따른 추가 영향은 없습니다.
- cs25-service/src/main/java/com/example/cs25service/domain/quiz/service/QuizAccuracyCalculateService.java (라인 45)
- cs25-batch/src/main/java/com/example/cs25batch/batch/service/TodayQuizService.java (라인 48)
추가 조치 없이 머지 가능합니다.
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
3 participants
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.
🔎 작업 내용
🧩 트러블 슈팅
Long 타입을 조회하면서 타입이 불일치해서 발생한 문제로 추정됨
close #363
Summary by CodeRabbit