Refactor/문제 이미지 등록 오류 및 문제 전체 수정 API 수정 #203
Conversation
개요문제 카테고리 데이터 구조를 List에서 Map으로 변경하고, 이미지 업로드 시 파일 유효성 검사를 추가하며, 관련 S3 예외 코드를 신규 추가하여 요청 DTO, 서비스 로직, 도메인 서비스 및 테스트를 일관되게 업데이트했습니다. 변경사항
예상 코드 리뷰 난이도🎯 2 (Simple) | ⏱️ ~12분 관련 가능성 있는 PR
추천 레이블
추천 리뷰어
시 🐰
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/org/ezcode/codetest/domain/problem/service/ProblemDomainService.java (2)
51-62:categories파라미터가null인 경우NullPointerException발생 가능
categories.keySet()호출 전에 null 체크가 없습니다. 호출자가 null을 전달하면 NPE가 발생합니다.🔎 null 방어 코드 추가 제안
public void updateCategoryAndSearchEngine(Problem problem, Map<String, String> categories) { problemCategoryRepository.deleteAllByProblemId(problem.getId()); + if (categories == null || categories.isEmpty()) { + return; + } + List<String> codes = new ArrayList<>(categories.keySet()); List<Category> categoryList = categoryRepository.findAllByCategoryCodeIn(codes);
65-84:createProblem에서도 동일한 null 체크 필요
updateCategoryAndSearchEngine과 동일하게categories가 null인 경우를 방어해야 합니다. 또한 두 메서드에서 카테고리 코드 추출 및 저장 로직이 중복되어 있습니다.🔎 null 체크 및 공통 로직 추출 제안
public Problem createProblem(Problem problem, Map<String, String> categories) { if (problemRepository.existsByTitleAndIsDeletedIsFalse(problem.getTitle())) { throw new ProblemException(ProblemExceptionCode.DUPLICATE_PROBLEM); } Problem savedProblem = problemRepository.save(problem); + if (categories == null || categories.isEmpty()) { + return savedProblem; + } + List<String> codes = new ArrayList<>(categories.keySet());공통 로직 추출 (선택사항):
private void saveProblemCategories(Problem problem, Map<String, String> categories) { if (categories == null || categories.isEmpty()) { return; } List<String> codes = new ArrayList<>(categories.keySet()); List<Category> categoryList = categoryRepository.findAllByCategoryCodeIn(codes); List<ProblemCategory> problemCategories = categoryList.stream() .map(cat -> ProblemCategory.from(problem, cat)) .toList(); problemCategoryRepository.saveAll(problemCategories); }
🧹 Nitpick comments (1)
src/test/java/org/ezcode/codetest/application/problem/service/ProblemServiceTest.java (1)
195-204:addImageToExistingProblem의 새로운 검증 로직에 대한 테스트 추가 권장
ProblemService.addImageToExistingProblem에 추가된 null/empty 이미지 검증 로직에 대한 테스트 케이스가 없습니다.🔎 테스트 케이스 추가 예시
@Test @DisplayName("이미지 추가 시 빈 파일이면 예외 발생") void addImageToExistingProblem_shouldThrowExceptionWhenImageEmpty() { // given MultipartFile emptyFile = mock(MultipartFile.class); when(emptyFile.isEmpty()).thenReturn(true); // when & then assertThrows(S3Exception.class, () -> problemService.addImageToExistingProblem(1L, emptyFile)); } @Test @DisplayName("이미지 추가 시 null 파일이면 예외 발생") void addImageToExistingProblem_shouldThrowExceptionWhenImageNull() { // when & then assertThrows(S3Exception.class, () -> problemService.addImageToExistingProblem(1L, null)); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/org/ezcode/codetest/application/problem/dto/request/ProblemUpdateRequest.javasrc/main/java/org/ezcode/codetest/application/problem/service/ProblemService.javasrc/main/java/org/ezcode/codetest/domain/problem/service/ProblemDomainService.javasrc/main/java/org/ezcode/codetest/infrastructure/s3/exception/code/S3ExceptionCode.javasrc/test/java/org/ezcode/codetest/application/problem/service/ProblemServiceTest.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-14T14:33:58.372Z
Learnt from: Kimminu7
Repo: ezcode-my/backend PR: 63
File: src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/problem/ProblemQueryRepositoryImpl.java:24-40
Timestamp: 2025-06-14T14:33:58.372Z
Learning: ProblemController에서 ProblemSearchCondition 객체는 항상 new ProblemSearchCondition(category, difficulty)로 유효한 인스턴스를 생성해서 전달하므로, ProblemQueryRepositoryImpl의 searchByCondition 메서드에서 searchCondition 파라미터 자체에 대한 null 체크는 불필요하다. category와 difficulty 필드만 각각 null일 수 있다.
Applied to files:
src/main/java/org/ezcode/codetest/domain/problem/service/ProblemDomainService.java
📚 Learning: 2025-07-02T12:05:54.917Z
Learnt from: Kimminu7
Repo: ezcode-my/backend PR: 133
File: src/test/java/org/ezcode/codetest/domain/problem/service/ProblemDomainServiceTest.java:92-99
Timestamp: 2025-07-02T12:05:54.917Z
Learning: ProblemDomainService의 removeProblem 메서드는 DB에서 Problem을 삭제한 후 Elasticsearch에서도 해당 ProblemSearchDocument를 찾아서 삭제합니다. 만약 Elasticsearch에서 문서를 찾지 못하면 ProblemException(PROBLEM_NOT_FOUND)을 던지므로, 테스트에서는 problem.getId()와 searchRepository.findById() 모두 적절하게 mock해야 합니다.
Applied to files:
src/test/java/org/ezcode/codetest/application/problem/service/ProblemServiceTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
src/main/java/org/ezcode/codetest/infrastructure/s3/exception/code/S3ExceptionCode.java (1)
16-17: LGTM!새로운
S3_FILE_EMPTY상수가 기존 패턴을 잘 따르고 있으며, 클라이언트 측 검증 오류에 대해HttpStatus.BAD_REQUEST를 적절하게 사용하고 있습니다.src/main/java/org/ezcode/codetest/application/problem/dto/request/ProblemUpdateRequest.java (1)
14-16: Map의 값(value)이 사용되지 않음 - 의도 확인 필요
categories필드가Map<String, String>으로 변경되었지만,ProblemDomainService에서는 키(코드)만 추출하여 사용하고 있습니다. Map의 값(한글 이름)은 사용되지 않습니다.의도적인 설계인지 확인이 필요합니다:
- API 문서화/가독성 목적이라면 현재 구조가 적절합니다.
- 값을 사용할 계획이 없다면
List<String>이 더 간결할 수 있습니다.또한 Swagger example 형식이 JSON Map 구문과 다릅니다.
🔎 Swagger example 수정 제안
- @Schema(description = "카테고리 코드 식별자(영어) / 한글 이름", example = "FOR_BEGINNER : 입문자용") + @Schema(description = "카테고리 코드 식별자(영어) / 한글 이름", example = "{\"FOR_BEGINNER\": \"입문자용\"}") Map<String, String> categories,src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java (2)
193-196: LGTM!이미지 파일의 null/empty 검증이 적절하게 추가되었습니다. 새로 추가된
S3_FILE_EMPTY예외 코드를 활용하여 명확한 오류 메시지를 제공합니다.
144-147: 불필요한 지적입니다.getImageUrl()은 null을 반환할 수 없습니다.
imageUrl필드는 선언 시점에new ArrayList<>()로 초기화되며, 어디서도 null로 설정되지 않습니다. 또한 Problem 엔티티의addImage()(150줄),clearImages()(155줄) 메서드에서도 imageUrl에 대해 직접 메서드를 호출하고 있으므로, 개발자들도 이 필드가 null이 아님을 알고 있습니다.@ElementCollectionJPA 어노테이션도 컬렉션을 항상 초기화된 상태로 유지합니다. 코드베이스의 다른 부분(ProblemDetailResponse 74줄)에서도 동일한 패턴으로 null 체크 없이isEmpty()를 호출하고 있습니다.src/test/java/org/ezcode/codetest/application/problem/service/ProblemServiceTest.java (1)
166-172: LGTM!테스트가
ProblemDomainService.updateCategoryAndSearchEngine의 변경된 시그니처(Map<String, String>)에 맞게 적절히 업데이트되었습니다.
작업 내용
변경 사항
UserService.createUser()메서드 추가@Email유효성 검증 적용트러블 슈팅
@Transactional이 적용되지 않음this.→AopProxyUtils.사용)해결해야 할 문제
UserController에서 비즈니스 로직 일부 처리 → 서비스로 이전 고려 필요참고 사항
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit
릴리스 노트
버그 수정
개선사항
✏️ Tip: You can customize this high-level summary in your review settings.