Conversation
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
코드 잘 봤습니다!! 전체적으로 코드가 가독성이 좋아서 리뷰하기 수월했던 것 같습니다~ 👍🏻 👍🏻
| private CommentCountUpdatable findPost(PostType type, Long postId) { | ||
| return switch (type) { | ||
| case FEED -> feedCommandPort.getByIdOrThrow(postId); | ||
| case RECORD -> recordCommandPort.getByIdOrThrow(postId); | ||
| case VOTE -> voteCommandPort.getByIdOrThrow(postId); | ||
| }; | ||
| } |
There was a problem hiding this comment.
공통 인터페이스를 사용하니 확실히 코드가 간단해지네여~
| public static Comment createComment(String content, Long postId, Long creatorId, String type, | ||
| boolean isReplyRequest, Long parentId, Comment parent) { | ||
|
|
||
| // 댓글/답글 생성 검증 | ||
| validateCommentCreate(isReplyRequest,parentId); | ||
| PostType postType = PostType.from(type); | ||
|
|
||
| if (isReplyRequest) { | ||
| // 답글 생성 검증 | ||
| validateReplyCommentCreate(postId, parent); | ||
| return withoutIdReplyComment(content, postId, creatorId, parent, postType); | ||
| } | ||
| return withoutIdRootComment(content, postId, creatorId, postType); | ||
| } |
There was a problem hiding this comment.
내부에서 팩토리 메서드를 통해 잘 구현하신 것 같아요!! LGTM!!
| @Test | ||
| @DisplayName("각 게시물 타입별로 존재하는 게시물에 대해 (루트)댓글 생성을 할 수 있다.") | ||
| void createRootCommentEachPostType() throws Exception { | ||
|
|
||
| // given | ||
| String[] postTypes = {"feed", "record", "vote"}; | ||
| Long[] postIds = {feed.getPostId(), record.getPostId(), vote.getPostId()}; | ||
|
|
||
| // when & then | ||
| for (int i = 0; i < postTypes.length; i++) { | ||
| mockMvc.perform(post("/comments/{postId}", postIds[i]) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(toJson("루트 댓글입니다", false, null, postTypes[i])) | ||
| .requestAttr("userId", user.getUserId())) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.data.commentId").exists()); | ||
| } | ||
| } |
There was a problem hiding this comment.
p3: postTypes에 들어가는 상수들은 운영 코드에서 PostType으로 이미 정의되어 있는 상태이니 이를 활용해보는건 어떨까요?
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
뒤에 PR 리뷰하다가 갑자기 생각나서 추가적으로 리뷰 하나 남겼습니다!
src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java
Show resolved
Hide resolved
![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
🧹 Nitpick comments (2)
src/main/java/konkuk/thip/room/domain/service/RoomParticipantService.java (2)
16-16: 메서드 파라미터 순서 검토 제안메서드명이 "사용자가 방 멤버인지 검증"을 의미하므로
validateUserIsRoomMember(Long userId, Long roomId)로 순서를 변경하는 것이 더 직관적일 수 있습니다. 또는 내부 포트 호출 순서와 일치시키기 위해 현재 순서를 유지하는 것도 좋습니다.- public void validateUserIsRoomMember(Long roomId, Long userId) { - if (!participantPort.existByUserIdAndRoomId(roomId, userId)) { + public void validateUserIsRoomMember(Long userId, Long roomId) { + if (!participantPort.existByUserIdAndRoomId(roomId, userId)) {
17-20: 예외 처리 구조 개선 제안현재
IllegalArgumentException을 cause로 사용하고 있는데, 이는 입력 파라미터 문제가 아니라 비즈니스 규칙 위반이므로 적절하지 않을 수 있습니다. 또한 사용자 ID를 예외 메시지에 직접 노출하는 것은 보안상 검토가 필요합니다.- throw new InvalidStateException(ROOM_ACCESS_FORBIDDEN, - new IllegalArgumentException("사용자가 이 방의 참가자가 아닙니다. roomId=" + roomId + ", userId=" + userId)); + throw new InvalidStateException(ROOM_ACCESS_FORBIDDEN, + "사용자가 이 방의 참가자가 아닙니다. roomId=" + roomId);또는 로깅으로 디버그 정보를 남기고 예외 메시지는 간소화하는 방법도 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(3 hunks)src/main/java/konkuk/thip/common/post/CommentCountUpdatable.java(1 hunks)src/main/java/konkuk/thip/feed/domain/Feed.java(4 hunks)src/main/java/konkuk/thip/room/domain/service/RoomParticipantService.java(1 hunks)src/test/java/konkuk/thip/comment/adapter/in/web/CommentControllerTest.java(1 hunks)src/test/java/konkuk/thip/comment/adapter/in/web/CommentCreateAPITest.java(1 hunks)src/test/java/konkuk/thip/common/util/TestEntityFactory.java(4 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateControllerTest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomCloseJoinApiTest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomGetHomeJoinedRoomsApiTest.java(3 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomGetMemberListApiTest.java(2 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java(2 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/test/java/konkuk/thip/room/adapter/in/web/RoomCloseJoinApiTest.java
- src/test/java/konkuk/thip/room/adapter/in/web/RoomGetHomeJoinedRoomsApiTest.java
- src/test/java/konkuk/thip/room/adapter/in/web/RoomGetMemberListApiTest.java
- src/test/java/konkuk/thip/comment/adapter/in/web/CommentCreateAPITest.java
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/konkuk/thip/feed/domain/Feed.java
- src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java
- src/test/java/konkuk/thip/comment/adapter/in/web/CommentControllerTest.java
- src/test/java/konkuk/thip/common/util/TestEntityFactory.java
- src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: hd0rable
PR: THIP-TextHip/THIP-Server#101
File: src/test/java/konkuk/thip/comment/adapter/in/web/CommentControllerTest.java:118-265
Timestamp: 2025-07-23T17:41:55.507Z
Learning: CommentControllerTest는 댓글 생성 API의 검증 로직과 예외 상황만을 테스트하는 단위 테스트이며, 성공 케이스는 별도의 통합 테스트(CommentCreateAPITest)에서 다룬다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서 Record와 Vote는 Room에 속하지만 Feed는 Room에 속하지 않는 구조이며, 댓글 작성 시 Record/Vote에 대해서만 사용자가 해당 Room의 참가자인지 검증이 필요하다.
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (1)
Learnt from: seongjunnoh
PR: #101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
src/main/java/konkuk/thip/common/post/CommentCountUpdatable.java (2)
Learnt from: seongjunnoh
PR: #101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서 Record와 Vote는 Room에 속하지만 Feed는 Room에 속하지 않는 구조이며, 댓글 작성 시 Record/Vote에 대해서만 사용자가 해당 Room의 참가자인지 검증이 필요하다.
Learnt from: buzz0331
PR: #75
File: src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryRepositoryImpl.java:50-83
Timestamp: 2025-07-14T14:19:38.796Z
Learning: Vote와 VoteItem 엔티티는 자주 함께 사용되므로, N+1 문제를 방지하기 위해 양방향 매핑과 fetch join을 고려하는 것이 좋습니다. 특히 기록장 조회 API 등에서도 함께 사용될 가능성이 높습니다.
src/main/java/konkuk/thip/room/domain/service/RoomParticipantService.java (1)
Learnt from: seongjunnoh
PR: #101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
🧬 Code Graph Analysis (1)
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(28-240)
🔇 Additional comments (7)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateControllerTest.java (1)
121-122: 에러 코드 변경이 적절하게 반영되었습니다.
FEED_UPDATE_FORBIDDEN에서FEED_ACCESS_FORBIDDEN으로의 에러 코드 변경과 메시지 업데이트가 올바르게 테스트에 반영되었습니다. 더 일반적인 접근 권한 에러 코드를 사용함으로써 댓글 작성 등 다른 기능에서도 재사용 가능한 구조로 개선된 것으로 보입니다.src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (3)
66-66: 팩토리 메서드 이름 수정이 올바르게 적용되었습니다.
TestEntityFactory.createRoomParticipant메서드 호출로 수정되어 일관성이 향상되었습니다.
76-77: 참여자 생성 로직이 일관되게 수정되었습니다.방장과 일반 참여자 모두에 대해 올바른 팩토리 메서드를 사용하여 역할별로 적절히 구분되어 있습니다.
59-78: 테스트 설정 메서드들의 일관성이 개선되었습니다.다른 방 관련 테스트 클래스들과 동일한 팩토리 메서드를 사용하도록 수정되어 코드베이스 전반의 일관성이 향상되었습니다. 기존 테스트 로직은 그대로 유지되면서 메서드 이름만 올바르게 수정되었습니다.
src/main/java/konkuk/thip/room/domain/service/RoomParticipantService.java (1)
10-13: 도메인 서비스 구조가 잘 설계되었습니다.Spring Component로 적절히 등록되고 생성자 주입을 사용하여 의존성을 관리하고 있습니다. 방 참가자 검증 로직을 도메인 서비스로 캡슐화하여 재사용성을 높인 설계가 좋습니다.
src/main/java/konkuk/thip/common/post/CommentCountUpdatable.java (2)
5-5: getRoomId() 호출 경로 및 FEED 처리 검증 완료
- CommentCreateService에서
post.getRoomId()는 FEED가 아닐 때(PostType != FEED)에만 호출됩니다.- FEED인 경우에는
feed.validateCreateComment(userId)로 별도 검증 로직이 수행되므로, FEED의getRoomId()가null을 반환해도 호출되지 않아 문제가 없습니다.
3-6: CommentCountUpdatable 인터페이스 설계 적합 – getRoomId()는 RECORD/VOTE에만 사용됩니다.createComment 흐름을 확인한 결과,
roomParticipantService.validateUserIsRoomMember(post.getRoomId(), userId)호출은 오직 PostType.RECORD 혹은 VOTE일 때만 발생- PostType.FEED인 경우에는
getRoomId()가 아닌Feed.validateCreateComment(userId)로 처리따라서 FEED 구현체에
getRoomId()가 존재하더라도 실제 코드 흐름에서 호출되지 않아 문제되지 않습니다.
| @@ -0,0 +1,22 @@ | |||
| package konkuk.thip.room.domain.service; | |||
| @Override | ||
| //Feed는 RoomId 없음 | ||
| public Long getRoomId() { | ||
| return null; | ||
| } |
| private void validateCommentCreateAuthorization(PostType type, CommentCountUpdatable post, Long userId) { | ||
| // 2-1. RECORD, VOTE는 방 멤버 자격 검증 필요 | ||
| if (type == PostType.RECORD || type == PostType.VOTE) { | ||
| roomParticipantService.validateUserIsRoomMember(post.getRoomId(), userId); | ||
| } | ||
| // 2-2. FEED는 비공개 글 일시, 작성자 자격 검증 필요 | ||
| else { | ||
| Feed feed = (Feed) post; | ||
| feed.validateCreateComment(userId); | ||
| } | ||
| } |
| @ActiveProfiles("test") | ||
| @AutoConfigureMockMvc(addFilters = false) | ||
| @Transactional | ||
| @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD) |
There was a problem hiding this comment.
p3 : 저희 각 테스트마다 @AfterEach 로 테스트 DB deleteAllInBatch 하는데도 각 테스트끼리 영향을 받나요??
어떤 이슈가 있어서 해당 메서드 추가하셨는지 궁금합니다!
There was a problem hiding this comment.
아 이게 @transactional을 사용한 테스트코드에서는 @AfterEach없이 영향을 받아서 전체테스트에서는 실패하길래 독립성 보장을위해 추가했습니닷
There was a problem hiding this comment.
앗 테스트 클래스에 트랜잭션을 사용하셨군요!! 뭐 상관없습니다ㅎ
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
이와 관련해서 생기는 문제점에 대해 도입한 공통 인터페이스에 대해 문서화 댓글 생성 시 게시물의 댓글 수 증가 처리 해두었으니 해당 문서 보시고 코드 리뷰하기에 더 편할것같습니다!
Controller에서 Request의 Command 변환뒤 서비스로 요청이들어옴 ->
@DiscriminatorColumn(name = "dtype")으로 자동생성되는 칼럼과 중복 될 것같아 수정하지않았습니다.📸 스크린샷
💬 리뷰 요구사항
Feed의 update시에 관련된 tag,content를 무조건 제거하고 다시 덮어씌우는식으로하고있는데 나중에 변경감지하여 변경된 필드만 바꾸는 전략을 사용해봐도 좋을 것같습니다. (아직 잘 찾아보진 않았지만) 개발 속도를 높이기 위해서 해당 사항은 배제하고 개발했습니다 ㅠ 후에 리펙 진행하겠습니다.
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
버그 수정
테스트
기타