refactor: AllAnalysisCompletedHandler 메서드 책임 분리 및 N+1 쿼리 개선#69
Conversation
분리 및 N+1 쿼리 개선 (#68)
Boyeon-Shin
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the AllAnalysisCompletedHandler to improve modularity and performance, notably by optimizing database interactions through batch fetching of answers and feedbacks. The review feedback points out a critical issue where the @transactional annotation on the handle method might prevent session failure states from being saved due to transaction rollbacks. Additionally, it suggests using a merge function in Collectors.toMap to prevent potential exceptions and recommends reusing already loaded objects to avoid redundant database lookups during error handling.
| @@ -48,92 +50,133 @@ public class AllAnalysisCompletedHandler { | |||
| @Transactional | |||
There was a problem hiding this comment.
handle 메서드에 @Transactional이 선언되어 있어, 내부 로직 수행 중 런타임 예외가 발생하면 해당 트랜잭션은 rollback-only로 마킹됩니다. 이 경우 catch 블록에서 호출하는 failSession 메서드 내의 DB 저장 로직(sessionRepository.save)이 정상적으로 반영되지 않아, 세션 상태가 FAILED로 변경되지 않는 문제가 발생할 수 있습니다. 비즈니스 로직의 실패 여부와 관계없이 세션 상태를 업데이트하려면 트랜잭션 경계를 적절히 분리하거나 failSession을 별도의 트랜잭션(예: Propagation.REQUIRES_NEW)으로 처리하는 것을 권장합니다.
| List<InterviewQuestion> questions = questionRepository.findByInterviewSessionId(sessionId); | ||
|
|
||
| Map<UUID, InterviewAnswer> answerMap = answerRepository.findBySessionId(sessionId).stream() | ||
| .collect(Collectors.toMap(a -> a.getInterviewQuestion().getId(), a -> a)); |
There was a problem hiding this comment.
Collectors.toMap 사용 시 중복된 키(동일한 questionId를 가진 답변)가 존재할 경우 IllegalStateException이 발생할 수 있습니다. 현재 비즈니스 로직상 질문당 하나의 답변만 존재한다고 가정하더라도, 예기치 못한 데이터 중복 상황에 대비하여 merge function을 추가하는 것이 안전합니다.
| .collect(Collectors.toMap(a -> a.getInterviewQuestion().getId(), a -> a)); | |
| .collect(Collectors.toMap(a -> a.getInterviewQuestion().getId(), a -> a, (existing, replacement) -> existing)); |
| } | ||
|
|
||
| private void failSession(UUID answerId, String message) { | ||
| InterviewAnswer answer = answerRepository.findByIdWithQuestionAndSession(answerId).orElse(null); |
There was a problem hiding this comment.
failSession 메서드에서 answerRepository.findByIdWithQuestionAndSession(answerId)를 호출하여 세션 정보를 조회하고 있습니다. handle 메서드에서 이미 answer 객체를 조회한 상태라면 이를 재사용하여 불필요한 DB 조회를 줄일 수 있습니다. 또한, answerId가 유효하지 않을 경우 세션 실패 처리가 누락될 수 있으므로, 이벤트 객체에 sessionId를 포함하거나 조회 방식을 개선하는 것을 고려해 보세요.
1 participant
분리 및 N+1 쿼리 개선 (#68)
📌 관련 이슈 (Related Issue)
📝 작업 내용 (Description)
AllAnalysisCompletedHandler 메서드 책임 분리 및 N+1 쿼리 개선
🔄 변경 유형 (Type of Change)
✅ 체크리스트 (Checklist)