feat: 화상 면접 재시도 기능 구현 (#83)#85
Conversation
![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 introduces endpoints and service logic to create new interview sessions by cloning existing sessions or specific questions. Key feedback highlights the potential risk of creating empty sessions when no questions exist, performance concerns regarding database saves inside stream operations (suggesting the use of saveAll instead), and recommendations for more RESTful API path designs for the new endpoints.
| List<InterviewQuestion> oldQuestions = questionRepository.findByInterviewSessionIdOrderByCreatedAtAsc(oldSessionId); | ||
| InterviewSession newSession = sessionRepository.save(new InterviewSession(memberId, oldQuestions.size())); |
There was a problem hiding this comment.
이전 세션(oldSessionId)에 등록된 질문이 없는 경우(oldQuestions.isEmpty()), 질문이 0개인 빈 세션이 생성될 수 있습니다. 면접 세션은 최소 한 개 이상의 질문을 가져야 하므로, 질문 목록이 비어있는지 검증하는 로직을 추가하는 것이 안전합니다.
List<InterviewQuestion> oldQuestions = questionRepository.findByInterviewSessionIdOrderByCreatedAtAsc(oldSessionId);
if (oldQuestions.isEmpty()) {
throw new BusinessException(ErrorCode.QUESTION_NOT_FOUND);
}
InterviewSession newSession = sessionRepository.save(new InterviewSession(memberId, oldQuestions.size()));| List<UUID> newQuestionIds = oldQuestions.stream() | ||
| .map(q -> cloneQuestion(q, newSession).getId()) | ||
| .toList(); |
There was a problem hiding this comment.
스트림(stream()) 내부에서 데이터베이스에 변경을 주는 cloneQuestion (내부적으로 save 호출)을 호출하는 것은 부작용(Side-effect)을 유발하므로 지양해야 합니다. 또한, 여러 개의 질문을 개별적으로 save하는 대신 saveAll을 사용하여 일괄 저장(Batch Insert)하는 것이 성능과 가독성 측면에서 더 좋습니다.
다음과 같이 saveAll을 사용하도록 리팩토링하는 것을 제안합니다.
List<InterviewQuestion> newQuestions = oldQuestions.stream()
.map(q -> new InterviewQuestion(
q.getMemberId(),
q.getQuestion(),
q.getQuestionType(),
q.getQuestionTag(),
q.getJobPosting(),
newSession
))
.toList();
List<UUID> newQuestionIds = questionRepository.saveAll(newQuestions).stream()
.map(InterviewQuestion::getId)
.toList();| @PostMapping("/{sessionId}") | ||
| @ResponseStatus(HttpStatus.CREATED) | ||
| public ApiResponse<CreateSessionResponse> createSession( | ||
| @PathVariable UUID sessionId, | ||
| @RequestHeader("X-User-Id") UUID memberId | ||
| ) { | ||
| CreateSessionResponse data = sessionService.createSession(sessionId, memberId); | ||
| return ApiResponse.created(data, "세션이 생성되었습니다."); | ||
| } | ||
|
|
||
| @PostMapping("/{sessionId}/questions/{questionId}") | ||
| @ResponseStatus(HttpStatus.CREATED) | ||
| public ApiResponse<CreateSessionResponse> createQuestion( | ||
| @PathVariable UUID sessionId, | ||
| @PathVariable UUID questionId, | ||
| @RequestHeader("X-User-Id") UUID memberId | ||
| ) { | ||
| CreateSessionResponse data = sessionService.createQuestion(sessionId, questionId, memberId); | ||
| return ApiResponse.created(data, "세션이 생성되었습니다."); | ||
| } |
There was a problem hiding this comment.
현재 API 경로 설계에서 POST /api/interviews/sessions/{sessionId}와 POST /api/interviews/sessions/{sessionId}/questions/{questionId}는 기존 세션의 정보를 바탕으로 새로운 세션을 생성하고 있습니다.
하지만 이 경로는 기존 세션 리소스를 수정하거나 기존 세션 하위에 질문을 생성하는 것처럼 오해를 불러일으킬 수 있어 RESTful한 설계 관점에서 아쉬움이 있습니다.
다음과 같은 대안을 고려해 보시는 것을 추천합니다:
- 재시도(Retry) 명시:
POST /api/interviews/sessions/{sessionId}/retry및POST /api/interviews/sessions/{sessionId}/questions/{questionId}/retry와 같이 행위(retry)를 경로에 명시합니다. - 세션 생성 API 통합:
POST /api/interviews/sessions엔드포인트에 요청 바디(DTO)로parentSessionId와 선택적으로questionId를 전달하여 새로운 세션을 생성하도록 단일화합니다.
1 participant
📌 관련 이슈 (Related Issue)
📝 작업 내용 (Description)
화상 면접 재시도 기능 구현
🔄 변경 유형 (Type of Change)
✅ 체크리스트 (Checklist)