특정 다이어리 댓글 전체 조회(커서 기반) API 구현

[dnzp75]

작업 사항

• 특정 다이어리 댓글 전체 조회 API 구현
• 단위 테스트 작성

세부 작업 사항

• 커서 기반 페이지네이션 방식 사용
• 커서 값으로는 commentId 사용

구현 시 고민 사항

요구 사항

• 생성일 기준으로 댓글을 최신순으로 조회해야 함
• 커서 기반 페이지네이션을 사용하려면 커서 값을 요청으로 받아야 하며, 어떤 값을 커서로 사용할지 결정해야 함
• 최초에는 정렬 기준이 createdAt이므로 이를 커서로 고려했음

SELECT *
FROM comment
WHERE diary_id = ?
  AND created_at < ?
ORDER BY created_at DESC
LIMIT ?

문제점

• createdAt은 중복될 수 있기 때문에 커서 조건(WHERE)에 넣으면 중복된 createdAt을 가진 데이터가 누락될 수 있음
• 따라서 데이터 누락 방지를 위해 커서 기반 페이징의 WHERE 조건에는 유니크한 값을 사용하는 것이 일반적

해결 방법

• 현재 id는 @GeneratedValue(strategy = GenerationType.IDENTITY)로 자동 증가되고 있음
• 즉, 댓글 생성 시 commentId는 createdAt과 동일한 정렬 순서를 가지므로, commentId를 커서로 사용해도 무방함
• 이에 따라 아래와 같이 쿼리를 수정함

SELECT *
FROM comment
WHERE diary_id = ?
  AND (? IS NULL OR comment_id < ?)
ORDER BY comment_id DESC
LIMIT ?

개선 논의 사항

CommentResponseDto, PageResponse 등 여러 응답 DTO가 생기면서 DTO 네이밍이 유사하게 겹치는 현상이 있음

public PageResponse<CommentResponseDto> getCommentListByDiary(...) { ... }

향후 유지보수나 도메인 확장 시 혼동될 여지가 있기 때문에 DTO 네이밍 전략 개선 논의가 필요할 것 같습니다

[TTaiJin]

잘 작성해 주신 거 같습니다! 고생하셨습니다!

[sapiens2000]

테스트 수행 위해 잠시 닫았다가 열겠습니다.

[TTaiJin]

질문 남깁니다!

[TTaiJin]

다른 도메인간의 협력이 필요한 복잡한 유효성 검증 로직은 서비스 레이어에 두고 간단한 유효성 검증(ex 댓글 작성자 본인 확인, 다이어리 작성자 본인 확인 등)은 해당 엔티티에 두는 것에 대해 어떻게 생각하시는지 궁금합니다.

[dnzp75]

"댓글의 소유자는 나다"라는 도메인의 책임과 행위를 객체에게 위임한다고 생각했을 때에 지금보다 더 객체지향적인 설계가 될 수 있을 것 같아요.

	private void validateCommentOwner(Long userId, Comment comment) {
		if (!comment.getUserId().equals(userId)) {
			throw new UnauthorizedAccessException();
		}
	}

만약 해당 함수 그대로 엔티티에 두면 도메인 내부에서 예외처리가 이루어지게 되는데, 이것 자체가 비즈니스 예외를 직접 던지는 것이 부담스러워 질 수도 있다는 의견도 있네요.

도메인은 순수한 모델이어야 한다는 관점

도메인은 애플리케이션 계층이나 인프라에 의존하지 않고, 오직 자신의 상태와 행위만 책임지는 순수한 객체여야 한다.

예외를 던지는 행위는
→ 어떤 흐름 제어에 참여하는 것
→ 이건 사실 애플리케이션 레이어(서비스) 의 책임이라는 의견

그래서 일부는 이렇게 말함:

• 도메인이 예외를 던지면 순수성을 해친다
• 도메인은 true/false만 반환하고, 예외는 서비스에서 판단해라

// 순수 도메인 방식
public boolean isOwner(Long userId) {
    return this.userId.equals(userId);
}

// 서비스에서 판단
if (!comment.isOwner(userId)) {
    throw new UnauthorizedAccessException();
}

근데 또 실무에서는 도메인 내부에서 비즈니스 예외를 직접 던지는 멘위의 방식도 많이 사용한다고 하는 것 같더라구요

이유

• 이건 도메인 스스로가 자신의 유효성을 책임지는 방식이고,
• 복잡한 로직일수록 → 이쪽이 더 응집력 있고 깔끔하게 느껴짐

그래서 도메인 내부에서 예외처리 왜 부담스러울까?

• 테스트가 까다로워질 수 있음
  • 도메인에서 예외를 던지면 → 도메인 단위 테스트도 예외를 assert 해야 함
• 개발자가 도메인 테스트 코드를 쓰는 걸 꺼릴 수도 있음
• 도메인 객체가 예외를 많이 던지기 시작하면 무겁고 복잡해짐 -> 핵심 로직과 흐름 제어가 섞이게 되니까

부담스러운 이유를 보니까 저는 도메인은 순수한 모델이어야 한다는 관점도 테스트 작성하는 입장에서 좋아보이는데, 어떻게 보시나요?
평소에 저도 고민했던 부분인데 질문주셔서 나름 공부하게 되었네요 감사합니다.

[TTaiJin]

상세한 답변 감사합니다!
자세히 읽어보고 생각을 좀 해보니 말씀하신대로 도메인 객체가 너무 무거워 질 수도 있다는 생각이 드네요.
isOwner정도로 가볍게 참/거짓만 판별하고 예외를 던지는건 서비스 레이어에서 하는게 좋아 보입니다!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

• 커서 기반 페이징을 통한 댓글 조회 기능 추가
• CommentRepositoryCustom 및 CommentRepositoryImpl을 통한 쿼리 최적화
• 테스트 코드 개선 및 확장

주요 이슈:

1. 사용자 인증 처리

   • 현재 CommentController에서 userId를 하드코딩하고 있습니다.
   • 제안: 적절한 인증 메커니즘을 구현하여 실제 로그인한 사용자의 ID를 사용하세요.

   // CommentController.java
   @PostMapping
   public ResponseEntity<CommentCreateResponseDto> addComment(@RequestBody @Valid CommentCreateRequestDto requestDto) {
       Long userId = SecurityContextHolder.getContext().getAuthentication().getPrincipal();
       CommentCreateResponseDto response = commentService.addComment(userId, requestDto);
       return ResponseEntity.ok(response);
   }

2. 예외 처리 개선

   • NotFoundCommentException과 UnauthorizedAccessException에 대한 전역 예외 처리가 필요합니다.
   • 제안: @ControllerAdvice를 사용하여 전역 예외 처리기를 구현하세요.

   @ControllerAdvice
   public class GlobalExceptionHandler {
       @ExceptionHandler(NotFoundCommentException.class)
       public ResponseEntity<ErrorResponse> handleNotFoundComment(NotFoundCommentException ex) {
           return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new ErrorResponse("Comment not found"));
       }
   
       @ExceptionHandler(UnauthorizedAccessException.class)
       public ResponseEntity<ErrorResponse> handleUnauthorizedAccess(UnauthorizedAccessException ex) {
           return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse("Unauthorized access"));
       }
   }

3. 성능 최적화

   • CommentRepositoryImpl에서 쿼리 성능을 더욱 개선할 수 있습니다.
   • 제안: 인덱스 사용을 최적화하고, 필요한 필드만 선택적으로 조회하세요.

   // CommentRepositoryImpl.java
   @Override
   public Slice<Comment> findByDiaryIdWithCursor(Long diaryId, Long cursorCommentId, Pageable pageable) {
       List<Comment> result = queryFactory
           .select(Projections.constructor(Comment.class,
               comment.commentId,
               comment.content,
               comment.userId,
               comment.diaryId))
           .from(comment)
           .where(
               comment.diaryId.eq(diaryId),
               cursorCommentId != null ? comment.commentId.lt(cursorCommentId) : null
           )
           .orderBy(comment.commentId.desc())
           .limit(pageable.getPageSize() + 1)
           .fetch();
   
       // ... 나머지 코드는 동일
   }

관련 파일에 대한 영향 분석:

• CommentController의 변경으로 인해 인증 관련 설정 파일(예: SecurityConfig)의 수정이 필요할 수 있습니다.
• CommentRepositoryImpl의 최적화로 인해 Comment 엔티티에 인덱스 추가가 필요할 수 있습니다.
• 새로운 예외 처리 방식으로 인해 기존의 예외 처리 로직이 있는 파일들을 검토하고 일관성 있게 수정해야 합니다.

전반적인 의견:
코드 품질이 전반적으로 향상되었으며, 특히 커서 기반 페이징 구현은 큰 개선점입니다. 다만, 보안과 예외 처리 측면에서 추가적인 개선이 필요합니다.