Skip to content

Comment: comment 모듈 추가 및 application, api 작성 #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
May 30, 2025

Conversation

seonghooni
Copy link
Contributor

@seonghooni seonghooni commented May 3, 2025

Pull Request

Issues

Description

  • api, application, driven/rdb, driving/web 모듈 추가

    • comment 및 reply에 대한 domain, excepltion, readmodel 모듈을 작성했습니다.
    • comment 및 reply에 대한 port를 작성했습니다.
    • comment 및 reply에 대한 entity, persistence를 작성했습니다.
  • postgresql migration script를 작성했습니다.

  • basePackage 컨벤션에 맞게 리팩토링 진행했습니다.

How Has This Been Tested?

Comment, Reply에 대한 Create, Update, Delete API를 실행한 후,
Read API를 실행시킨 결과입니다.

image

Additional Notes

현재 작성한 댓글 조회 기능에서 댓글과 대댓글을 10개씩 가져오는데,
이 과정에서 N+1 비효율이 발생합니다.

게시물에 대한 댓글을 최대 10개, 또한 각각의 댓글에 대한 대댓글을 최대 10개 불러와야 하지만,
QueryDSL에서 제공하는 기본 구문으로는 이 코드를 최적화 할 수 없습니다.

nettee.comment.application.service.CommentQueryService의 함수입니다.

public List<CommentDetail> getCommentsByBoardId(Long boardId) {
        var comments = commentQueryRepositoryPort.findPageByBoardId(boardId, 0, 10);

        // comment별로 reply를 10개씩 가져옴
        **// 현재 N+1 이므로, 최대 11(1+10)개의 쿼리를 발생시킴**
        var result = comments.stream()
            .map(comment -> {
                var replies = replyQueryRepositoryPort.findPageByCommentId(comment.id(), 0, 10);

                return CommentDetail.builder()
                    .id(comment.id())
                    .content(comment.content())
                    .status(comment.status())
                    .createdAt(comment.createdAt())
                    .updatedAt(comment.updatedAt())
                    .replies(replies)
                    .build();
            }).collect(Collectors.toList());

        return result;
    }

위 코드를 NativeQuery를 써서 최적화를 할지
vs
11개의 쿼리는 N+1 문제일지라도 예측 가능한 범위 내에 있기 때문에 그대로 둘지

에 대한 고민은 해봐야 할 것 같습니다.

@merge-simpson merge-simpson added review: required Reviews are required type: feature 새로운 기능 또는 기능적 개선 A new feature or enhancement in: comment Issues in the comment module labels May 5, 2025
@merge-simpson merge-simpson changed the title Comment - comment 모듈 추가 및 application, api 작성 Comment: comment 모듈 추가 및 application, api 작성 May 13, 2025
@merge-simpson merge-simpson added review: ing Reviews are in progress and taking time and removed review: required Reviews are required labels May 17, 2025
@merge-simpson merge-simpson added this to the 🚄 Jun 02, 2025 milestone May 23, 2025
var comments = commentQueryRepositoryPort.findPageByBoardId(boardId, 0, 10);

// comment별로 reply를 10개씩 가져옴
// 현재 N+1 이므로, 최대 11(1+10)개의 쿼리를 발생시킴
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 기능에 대한 최적화를 말씀해주셔서 저도 고민을 해볼 수 있어서 좋았습니다.
commentsId를 꺼낸 다음에, 한꺼번에 조회를 하는 방식은 어떤지 궁금합니다.
한꺼번에 조회를 한 다음 정리를 해주면 될 것 같다는 생각이 들었습니다.

List commentIds = comments.stream()
.map(Comment::getId)
.collect(Collectors.toList());

List replies = replyQueryRepositoryPort.findAllByCommentIdIn(commentIds);

Copy link
Contributor

@silberbullet silberbullet May 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 정말 좋은 고민이였습니다.

해당 기능에 현재 QueryDSL이 제공되는 문법에는 윈도우 함수 적용이 쉽지가 않습니다. 또한 댓글과 대댓글에 케이스는 하나의 댓글에 몇개의 대댓글을 가져올지 ROW 개수 제한도 포함되어야 합니다.

🤔 저는 선택적으로 Native Query를 사용해야 한다고 생각합니다!

수용님이 작성하신 부분에서 더 수정을 하자면

// limit 갯수 추가
List replies = replyQueryRepositoryPort.findAllByCommentIdIn(commentIds, 10);
SELECT *
FROM (
         SELECT *,
                ROW_NUMBER() OVER (PARTITION BY reply.comment_id ORDER BY reply.created_at ASC) as rn
         FROM reply
         WHERE reply.comment_id IN (:commentIds)
     ) ranked
WHERE rn <= (:limit)

이렇게 되면 하기 와 같이 DB에서 조회가 되며 (5개만 조회)

image

조회 후에는 Application 단에서 commentId에 맞게 매핑을 시켜주면 된다고 생각합니다.

Copy link
Contributor Author

@seonghooni seonghooni May 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음.. 결국 특정 DB에 종속되는 건데 ROW_NUMBER()가 대부분의 RDBMS가 지원한다고 하더라도
특정 버전 (ex. MySQL 8.0 미만)의 경우 ROW_NUMBER()를 지원하지 않습니다.

이런 경우는 배제하고 효율을 챙기는 게 좋을까요?? @merge-simpson @sweetykr7 @silberbullet @wch-os

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*참고

관련 논의는 'JSON 필드 사용: reply 첫 페이지에 대해서만 JSON 필드에 중복으로 관리' 방식을 시도하는 것으로 이야기되었습니다.

Copy link
Contributor

@sweetykr7 sweetykr7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

많은 작업을 하시느라 고생 많으셨습니다. 작업량이 상당하셨는데 중간 중간 고민하신 포인트들 덕분에 저도 고민해볼 수 있어서 좋았습니다~!!

Copy link
Contributor

@silberbullet silberbullet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 작업하시느라 고생하셨습니다!

현재 기본 동작들에는 문제가 없어 🚀 Approve 합니다.

다만, 코드 스타일 적으로 관련된 질의는 남겨두겠습니다!

var comments = commentQueryRepositoryPort.findPageByBoardId(boardId, 0, 10);

// comment별로 reply를 10개씩 가져옴
// 현재 N+1 이므로, 최대 11(1+10)개의 쿼리를 발생시킴
Copy link
Contributor

@silberbullet silberbullet May 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 정말 좋은 고민이였습니다.

해당 기능에 현재 QueryDSL이 제공되는 문법에는 윈도우 함수 적용이 쉽지가 않습니다. 또한 댓글과 대댓글에 케이스는 하나의 댓글에 몇개의 대댓글을 가져올지 ROW 개수 제한도 포함되어야 합니다.

🤔 저는 선택적으로 Native Query를 사용해야 한다고 생각합니다!

수용님이 작성하신 부분에서 더 수정을 하자면

// limit 갯수 추가
List replies = replyQueryRepositoryPort.findAllByCommentIdIn(commentIds, 10);
SELECT *
FROM (
         SELECT *,
                ROW_NUMBER() OVER (PARTITION BY reply.comment_id ORDER BY reply.created_at ASC) as rn
         FROM reply
         WHERE reply.comment_id IN (:commentIds)
     ) ranked
WHERE rn <= (:limit)

이렇게 되면 하기 와 같이 DB에서 조회가 되며 (5개만 조회)

image

조회 후에는 Application 단에서 commentId에 맞게 매핑을 시켜주면 된다고 생각합니다.

@merge-simpson merge-simpson added review: provided Reviews have been provided and removed review: ing Reviews are in progress and taking time labels May 27, 2025
@seonghooni seonghooni merged commit 901b3dc into main May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: comment Issues in the comment module review: provided Reviews have been provided type: feature 새로운 기능 또는 기능적 개선 A new feature or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SUB-ISSUE] Comment: 모듈별 베이스 패키지 구분 [MAIN] Comment, Reply Domain 구현
4 participants