Repository navigation
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| questions.clear(); | ||
| newQuestions.forEach(question -> question.setDailyQuiz(this)); | ||
| questions.addAll(newQuestions); | ||
| } |
There was a problem hiding this comment.
[HIGH] replaceQuestions()가 기존 문항을 소프트 삭제가 아닌 물리 삭제함
Problem: questions 컬렉션이 orphanRemoval = true로 매핑되어 있어, replaceQuestions()에서 questions.clear()를 호출하고 UpdateDailyQuizService에서 flush()를 실행하면 기존 DailyQuizQuestion 로우가 DB에서 즉시 물리 삭제됩니다. CLAUDE.md의 "Soft delete only — never physically delete a record. Set deleted = true instead." 규칙을 정면으로 위반합니다.
Evidence: DailyQuizQuestion에는 이미 softDelete() 메서드가 정의되어 있지만(DailyQuizQuestion.java), replaceQuestions()가 이를 우회하여 한 번도 호출되지 않는 죽은 코드가 됩니다. 관리자가 PATCH /admin/daily-quizzes/{id}로 문항을 수정할 때마다 기존 3개 문항 로우가 영구 삭제됩니다.
Fix direction: orphanRemoval = true를 제거하고, replaceQuestions()에서 제거 대상 문항에 대해 softDelete()를 호출한 뒤 컬렉션에서는 분리하는 방식으로 변경하세요.
public void replaceQuestions(List<DailyQuizQuestion> newQuestions) {
questions.forEach(DailyQuizQuestion::softDelete);
questions.clear();
newQuestions.forEach(question -> question.setDailyQuiz(this));
questions.addAll(newQuestions);
}There was a problem hiding this comment.
DailyQuizQuestion의 softDelete() 제거
DailyQuizQuestion은 DailyQuiz에 종속되어 있는 컬렉션으로 문항 단독으로 조회,참조되는 경로가 없고
다른 엔티티가 문항을 FK로 참조하는 경우도 없음
같은 성격의 종속 컬렉션인 Quiz.solveHistories도 이미 동일한 매핑을 사용하고 있으므로 현행 유지
| if (validatedQuestions != null) { | ||
| dailyQuiz.replaceQuestions(validatedQuestions); | ||
| dailyQuizRepository.flush(); | ||
| } |
There was a problem hiding this comment.
[HIGH] questions를 생략한 PATCH 요청 시 지연 컬렉션 접근으로 LazyInitializationException 발생 가능
Problem: request.getQuestions()가 null이면 validatedQuestions가 null로 남아 dailyQuiz.replaceQuestions(...)가 호출되지 않고, questions 지연 컬렉션이 트랜잭션 내에서 초기화되지 않습니다. 이후 컨트롤러의 toResponse()가 트랜잭션 종료 후 dailyQuiz.getQuestions().stream()을 호출합니다.
Evidence: Spring Boot 3.x는 spring.jpa.open-in-view 기본값이 false이고, 이 프로젝트에는 이를 재정의하는 설정이 없습니다. API 문서(UpdateDailyQuizController.java L37)에 명시된 "발행 전환만 하려면 {"published": true}만 전달" 시나리오가 정확히 이 경로를 타므로, 문서화된 정상 사용 흐름에서 예외가 발생합니다. 형제 서비스인 AdminReadDailyQuizService는 동일한 문제를 피하기 위해 dailyQuiz.getQuestions().size();로 명시적으로 즉시 초기화합니다.
Fix direction: 서비스 메서드 종료 전에 questions 여부와 관계없이 컬렉션을 초기화하세요.
dailyQuiz.getQuestions().size(); // force init before leaving @Transactional scope
return UpdateDailyQuizResponse.builder()
.dailyQuiz(dailyQuiz)
.build();
| @Override | ||
| public boolean isValid() { | ||
| return dailyQuizId != null; | ||
| } | ||
| } |
There was a problem hiding this comment.
빈 문자열과 변경 내용이 없는 PATCH 요청이 허용됩니다.
현재 isValid()에서는 dailyQuizId만 확인하고 있고, DailyQuiz.update()에서도 각 값이 null인지만 확인하고 있습니다. 따라서 {} 요청이나 {"topic": " "} 같은 요청도 정상 처리되며, 후자의 경우 생성 시에는 허용되지 않는 빈 값이 저장될 수 있습니다.!
수정할 필드가 하나 이상 존재하는지 확인하고, 전달된 topic, warmUp, 등 필드들이 빈 문자열이 아닌지도 함께 검증하는 것이 좋을 것 같습니다.
There was a problem hiding this comment.
좋은 리뷰 감사합니다 !
필드, 빈 문자열 검증 추가하는 방향으로 수정하였습니다.
| 요청 | 이전 | 지금 |
|---|---|---|
{"topic": ""} |
200 (빈 값 저장) | 400 |
{"topic": " "} |
200 (공백 저장) | 400 |
{} |
200 | 400 |
2 participants
연관 이슈
작업 사항
feat(#185): 5분 상식 퀴즈 도메인 엔티티 및 리포지토리 추가feat(#185): 5분 상식 퀴즈 세트 등록 API 구현feat(#185): 5분 상식 퀴즈 세트 수정 및 발행 전환 API 구현feat(#185): 5분 상식 퀴즈 세트 목록 조회 API 구현feat(#185): 5분 상식 퀴즈 세트 상세 조회 API 구현테스트
주의 사항 및 참고사항