Merged
Conversation
- Comment 엔티티의 parentId(long)를 parent(Comment 참조)로 변경
- 대댓글 생성을 위한 parent Comment 조회 로직 추가
- Repository 메서드명 모호성 해결 (findByParent_Id 사용)
- CommentReader의 getComments(), getComments1() 구현
- 대댓글 조회 API 엔드포인트 추가 (GET /api/comments/{parentId}/replies)
- 테스트 코드 parent 참조로 업데이트
- 4개 엔드포인트를 2개로 통합 (/api/articles, /api/articles/me) - QueryDSL BooleanBuilder로 동적 쿼리 구현 - 복합 필터 AND 조건 지원 (keyword + tag + categoryId) - ArticleQueryParam DTO로 파라미터 캡슐화 - MemberReader 의존성 제거로 순환 참조 방지 - 기존 8개 Repository 메서드를 1개로 통합
- OAuth 서비스 필드 선언을 클래스 상단으로 이동 - Article 도메인 불필요한 Javadoc 제거 - 불필요한 공백 라인 정리
|
Caution Review failedThe pull request is closed. Note
|
| Cohort / File(s) | Summary |
|---|---|
Article Search API 통합 ArticleController.java, ArticleService.java, ArticleReader.java, ArticleRepositoryCustom.java, ArticleRepositoryImpl.java, ArticleQueryParam.java |
다중 검색 메서드(getArticles, searchArticles 등)를 getArticles(Pageable, Long memberId, String keyword, String tag, Long categoryId)로 통합. 동적 쿼리 빌더 방식 도입. |
Comment 부모-자식 관계 리팩토링 Comment.java, CommentController.java, CommentCreateRequest.java, CommentRepository.java, CommentReader.java, CommentWriter.java |
parentId(long)에서 parent(Comment) @ManyToOne 관계로 변경. 새로운 /comments/{parentId}/replies 엔드포인트 추가. |
응답 DTO 개선 PageResponse.java, ErrorResponse.java, ArticleResponse.java |
PageResponse의 data 필드를 contents로 이름 변경; ErrorDetail 중첩 레코드 추가; ArticleResponse에 @Builder와 factory 메서드 추가. |
Minor 포맷팅 및 구조 정리 ErrorCode.java, GlobalExceptionHandler.java, AsyncConfig.java, Temp.java, AiService.java, TokenService.java, KakaoOAuthService.java, NaverOAuthService.java, CustomWritingStyle.java |
공백, 필드 순서, 불필요한 import 정리. 기능적 변화 없음. |
⚠️ 주요 검토 사항 (보안 & 비즈니스 로직)
1. LazyInitializationException 위험 - Comment 엔티티
Comment 엔티티의 parent 필드가 @ManyToOne(fetch = FetchType.LAZY)로 선언되었는데, CommentController의 새 엔드포인트 getReplies가 PageResponse<Reply>를 반환합니다.
문제점:
- Reply 또는 CommentArticleResponse DTO 직렬화 시, parent 필드가 LAZY 로드되지 않은 상태에서 접근하면
LazyInitializationException발생 - CommentReader의
getComments1메서드에서 댓글을 조회한 후 replies를 추가하는데, parent 필드를 접근하면 위험
권장사항:
// Comment 조회 시 parent를 eager load하거나
`@Query`("SELECT c FROM Comment c LEFT JOIN FETCH c.parent WHERE c.id = :id")
Comment findByIdWithParent(Long id);
// 또는 Reply/CommentArticleResponse DTO에서 parent 필드를 제외
// 또는 fetch = FetchType.EAGER로 변경 (N+1 주의)2. N+1 쿼리 문제 - ArticleRepositoryImpl.searchArticles
새로운 통합 searchArticles(ArticleQueryParam, Pageable) 메서드가 동적 쿼리 빌더(BooleanBuilder)를 사용합니다.
문제점:
findArticleIdsByTag(String tagName, Long memberId)헬퍼 메서드가 먼저 태그로 article ID를 조회- 이후 main query에서 해당 ID 목록으로 다시 조회 → 불필요한 쿼리 증가
- ArticleResponse.from() 호출 시 article.getTags() 접근 → 태그가 LAZY 로드되면 N+1 발생 가능
권장사항:
// 태그 필터링이 필요하면 JOIN을 활용한 단일 쿼리로 구성
// 또는 article 조회 시 tags를 LEFT JOIN FETCH로 eager load
`@Query`("SELECT DISTINCT a FROM Article a " +
"LEFT JOIN FETCH a.tags WHERE a.tags.name = :tagName")3. Authorization 우회 가능성 - Article 검색
ArticleQueryParam의 memberId는 선택적(null 가능)인데, ArticleRepositoryImpl의 동적 필터링에서 memberId 확인이 명확하지 않습니다.
문제점:
- 만약 memberId가 null이면 모든 사용자의 게시글이 반환
- 하지만 유저가 자신의 게시글만 조회하려 할 때
getMyArticles와getArticles간의 구분이 명확해야 함 - ArticleQueryParam.hasMemberFilter()는 memberId 존재 여부만 확인 → 값 검증 부재
권장사항:
// 컨트롤러에서 getMyArticles는 반드시 현재 사용자의 memberId를 강제
public ResponseEntity<...> getMyArticles(
`@MemberId` Long memberId, // 주입된 사용자 ID
`@RequestParam`(required = false) String keyword,
...
) {
// memberId 검증: null이면 401 Unauthorized
if (memberId == null) {
throw new UnauthorizedException("로그인이 필요합니다");
}
return articleService.getArticles(pageable, memberId, keyword, tag, categoryId);
}4. Edge Case - Comment 부모 검증 누락
CommentWriter에서 parentCommentId > 0일 때 부모 댓글을 조회하는데, 조회 실패 시 처리가 명시되지 않았습니다.
문제점:
if (request.parentCommentId() > 0) {
Comment parentComment = commentReader.getComment(request.parentCommentId());
// parentComment가 null이면? 또는 다른 게시글의 댓글이면?
return commentRepository.save(comment.parent(parentComment));
}- 부모 댓글이 삭제되었거나 존재하지 않으면 NullPointerException
- 부모 댓글이 다른 게시글의 댓글이라면? 유효성 검증 부재
권장사항:
if (request.parentCommentId() > 0) {
Comment parentComment = commentReader.getComment(request.parentCommentId());
if (parentComment == null) {
throw new CommentNotFound("부모 댓글을 찾을 수 없습니다");
}
// parentComment와 현재 article이 같은 게시글인지 확인
if (!parentComment.getArticle().getId().equals(article.getId())) {
throw new InvalidCommentHierarchy("다른 게시글의 댓글에 답글을 달 수 없습니다");
}
}5. CommentRepository 메서드 변경 - 기존 쿼리 호환성
기존 findByArticle_Id(articleId, pageable)가 제거되고 findByArticle_IdAndParentIsNull(articleId, pageable)로 변경되었습니다.
문제점:
- 만약 다른 부분의 코드(테스트 포함)에서
findByArticle_Id를 호출하면 컴파일 실패 - PR에서 보이는 변경사항은 테스트도 수정했으나, 혹시 빠진 사용처가 있을 수 있음
권장사항:
- 모든 레포지토리 메서드 호출처를 Grep으로 검증
- 만약
findByArticle_Id를 호출하는 코드가 남아있다면 명확한 오류 메시지 표시
6. Error Handling 개선 - ErrorResponse.ErrorDetail
새로운 ErrorDetail(String code, String message) 레코드가 추가되었으나, 기존 ErrorResponse와의 호환성 확인 필요합니다.
권장사항:
- 기존 에러 응답 형식이 변경되면, API 버전 관리 및 클라이언트 영향 검토 필요
- GlobalExceptionHandler에서 ErrorDetail을 사용하는 예제 코드 확인 필수
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
이유:
- 다중 엔티티 관계 변경 (Comment parent relationship)
- 쿼리 로직 통합으로 인한 N+1 위험성
- Authorization 및 검증 로직의 분산
- LazyInitializationException 위험 존재
- 여러 파일 간의 의존성 변화로 인한 복합적 영향
Possibly related PRs
- Refactor: 게시글, 알림 리팩토링 #59: 동일한 ArticleQueryParam 도입 및 검색 API 통합 로직 포함
- Refactor: 게시글 & 댓글 리팩토링 #53: Comment 부모-자식 관계 리팩토링 및 CommentController/Repository 변경 포함
- Refactor: 테스트 코드 작성 #60: ErrorResponse.ErrorDetail 중첩 레코드 추가 포함
Poem
검색을 통합하고 댓글의 족보를 정리하네 🌳
부모의 손을 놓고 관계로 삼으며,
동적 쿼리로 유연함을 얻었지만
LazyInitialization의 유령이 도사리... 👻
단, 먼저 Eager 손을 맞잡고 가자 🤝
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
dev
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 help to get the list of available commands and usage tips.
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
1 participant
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.
Summary by CodeRabbit
릴리스 노트
새로운 기능
리팩토링
data→contents)