Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/konkuk/thip/comment/domain/service/CommentAuthorizationService.java (1)
48-48: 불필요한 빈 줄을 제거하세요.-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java(1 hunks)src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java(1 hunks)src/main/java/konkuk/thip/comment/domain/policy/CommentAccessPolicy.java(1 hunks)src/main/java/konkuk/thip/comment/domain/policy/FeedCommentAccessPolicy.java(1 hunks)src/main/java/konkuk/thip/comment/domain/policy/RoomPostCommentAccessPolicy.java(1 hunks)src/main/java/konkuk/thip/comment/domain/service/CommentAuthorizationService.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/common/post/service/PostQueryService.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 (3)
- 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
🚧 Files skipped from review as they are similar to previous changes (8)
- src/main/java/konkuk/thip/common/post/CommentCountUpdatable.java
- src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java
- src/main/java/konkuk/thip/feed/domain/Feed.java
- src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java
- src/test/java/konkuk/thip/common/util/TestEntityFactory.java
- src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
- src/test/java/konkuk/thip/comment/adapter/in/web/CommentCreateAPITest.java
- src/test/java/konkuk/thip/comment/adapter/in/web/CommentControllerTest.java
🧰 Additional context used
🧠 Learnings (5)
📓 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)에서 다룬다.
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/service/PostQueryService.java (1)
Learnt from: seongjunnoh
PR: #43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
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 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
src/main/java/konkuk/thip/comment/domain/policy/RoomPostCommentAccessPolicy.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 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
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의 참가자인지 검증이 필요하다.
🧬 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(29-248)
⏰ 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 (15)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateControllerTest.java (1)
121-122: 에러 코드 리팩토링이 올바르게 반영되었습니다.
FEED_UPDATE_FORBIDDEN에서FEED_ACCESS_FORBIDDEN으로 변경된 에러 코드와 메시지가 테스트에 정확히 반영되어 있습니다. 더 일반적인 접근 권한 에러 코드를 사용하는 것이 API 일관성 측면에서 좋은 개선입니다.src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (2)
66-66: 팩토리 메서드 변경이 적절합니다.
TestEntityFactory.createRoomParticipant메서드로의 변경이 올바르게 적용되었습니다. HOST 역할과 0.0 userPercentage 파라미터가 적절하게 사용되었습니다.
77-77: 팩토리 메서드 변경이 일관성 있게 적용되었습니다.참여자 생성 시에도
TestEntityFactory.createRoomParticipant메서드가 올바르게 사용되었습니다. MEMBER 역할과 초기 userPercentage 설정이 적절합니다.src/main/java/konkuk/thip/comment/domain/policy/CommentAccessPolicy.java (1)
6-9: 잘 설계된 전략 패턴 인터페이스입니다.
CommentAccessPolicy인터페이스가 명확한 책임 분리와 함께 잘 설계되어 있습니다.supports()메서드로 정책 선택 로직을,validateCommentAccess()메서드로 실제 검증 로직을 분리한 설계가 적절합니다.src/main/java/konkuk/thip/comment/domain/policy/FeedCommentAccessPolicy.java (2)
12-14: FEED 타입 지원 로직이 올바르게 구현되었습니다.
PostType.FEED에 대해서만true를 반환하는 로직이 정확합니다.
17-20: 캐스팅 안전성 검증 완료supports(PostType.FEED)가 FeedCommentAccessPolicy에만 true를 반환하며,
CommentAuthorizationService 초기화 로직에서 FEED 타입에 FeedCommentAccessPolicy가 매핑됩니다.
또한 PostQueryService.findPost(PostType.FEED, …)의 반환 타입이 Feed이므로
(Feed) post캐스팅은 안전합니다. 별도 조치가 필요 없습니다.src/main/java/konkuk/thip/room/domain/service/RoomParticipantService.java (1)
10-22: 도메인 서비스가 잘 구현되었습니다.
RoomParticipantService가 "사용자가 방에 속하는지 검증" 로직을 적절히 캡슐화하고 있으며, 이는 학습된 프로젝트 패턴과 일치합니다. 포트-어댑터 패턴을 통한 데이터 접근과 명확한 예외 처리가 잘 구현되어 있습니다.src/main/java/konkuk/thip/common/post/service/PostQueryService.java (1)
19-25: PostType 모든 케이스 처리 및 getByIdOrThrow 메서드 확인 완료PostType(FEED, RECORD, VOTE)에 대한 모든 분기 처리가 switch 구문에 포함되어 있고, 각 CommandPort(feed, record, vote)에
getByIdOrThrow메서드가 정상 구현되어 있습니다.
추가 수정 없이 머지 가능합니다.src/main/java/konkuk/thip/comment/domain/policy/RoomPostCommentAccessPolicy.java (2)
16-18: RECORD와 VOTE 타입 지원 로직이 정확합니다.
PostType.RECORD와PostType.VOTE모두를 지원하는 로직이 올바르게 구현되어 있으며, 이는 두 타입 모두 Room에 속한다는 도메인 규칙과 일치합니다.
21-23: 방 참가자 검증 로직이 적절히 위임되었습니다.
RoomParticipantService를 통한 검증 로직 위임이 적절하며,CommentCountUpdatable인터페이스의getRoomId()메서드를 안전하게 사용하고 있습니다.src/main/java/konkuk/thip/comment/domain/service/CommentAuthorizationService.java (5)
1-16: import 구문이 적절하게 구성되었습니다.패키지 선언과 import 구문들이 깔끔하게 정리되어 있고, 모든 import가 실제로 사용되고 있습니다. static import 사용도 적절합니다.
17-22: 클래스 설계가 잘 되어 있습니다.@component 어노테이션과 private final 필드 사용이 적절하며, 정책 패턴을 위한 Map 구조도 효율적입니다.
35-38: private 검증 메서드가 잘 구현되었습니다.메서드의 책임이 명확하고, 적절한 추상화 수준을 유지하고 있습니다. 네이밍도 의도를 잘 표현하고 있습니다.
40-46: public 검증 메서드들이 잘 설계되었습니다.메서드 오버로딩을 통해 다양한 사용 사례를 지원하고 있으며, DRY 원칙을 잘 따르고 있습니다.
49-55: getPolicy 메서드가 적절하게 구현되었습니다.적절한 예외 처리와 함께 정책을 조회하는 로직이 깔끔하게 구현되어 있습니다.
src/main/java/konkuk/thip/comment/domain/service/CommentAuthorizationService.java
Outdated
Show resolved
Hide resolved
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
새로운 계층이 도입되면서 생각해야될게 많아졌네요 하핳
우선, 제가 생각하는 방향을 리뷰로 작성해두었는데 희진님이 생각하신 방향과 다르다면 답글 남겨주시면 감사하겠습니다~
| @Component | ||
| public class CommentAuthorizationService { | ||
|
|
||
| private final PostQueryService postQueryService; | ||
| private final Map<PostType, CommentAccessPolicy> policyMap; | ||
|
|
||
| public CommentAuthorizationService(PostQueryService postQueryService, List<CommentAccessPolicy> policies) { | ||
| this.postQueryService = postQueryService; | ||
| this.policyMap = new HashMap<>(); | ||
| for (CommentAccessPolicy policy : policies) { | ||
| for (PostType type : PostType.values()) { | ||
| if (policy.supports(type)) { | ||
| policyMap.put(type, policy); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
오호 List<전략 인터페이스>로 파라미터를 넣어 DI를 하면 모든 구현체들이 들어간다는 것은 처음 알았네요!
찾아보니 Spring에서 Map을 통해 전략 패턴을 적용하는 방식이 크게 두가지 인 것 같습니다.
- supports() 메서드 기반으로 런타임에 매핑 생성
• List를 주입받아 supports() 메서드를 확인하고 동적으로 Map을 생성하는 방식 - Configuration에서 Map Bean을 명시적으로 등록
• @configuration 클래스에서 Map<Key, Strategy>를 직접 정의한 뒤 주입받아 사용하는 방식
현재 저희 서비스 특성상 PostType이 Feed / Record / Vote 3가지로 고정적이고, 추가적인 도메인이 늘어날 가능성이 적기 때문에 supports 기반으로 동적으로 매핑하는 것보다 Configuration에서 명시적으로 Map을 Bean으로 등록하는 쪽이 PostType과 Plicy 간 매핑 관계를 한눈에 확인할 수 있기 때문에 가독성 및 유지보수 차원에서 더 좋을 것 같다는 생각입니다! 또한, CommentAuthorizationService의 책임도 단순해질 것 같습니다.
Cofiguration 방식을 적용하면 아래와 같은 코드로 도메인 서비스가 단순해질 것 같습니다.
@Configuration
@RequiredArgsConstructor
public class CommentAccessPolicyConfig {
private final FeedCommentAccessPolicy feedPolicy;
private final RoomPostCommentAccessPolicy roomPolicy;
@Bean
public Map<PostType, CommentAccessPolicy> commentAccessPolicyMap() {
return Map.of(
PostType.FEED, feedPolicy,
PostType.RECORD, roomPolicy,
PostType.VOTE, roomPolicy
);
}
}CommentAuthorizationService에서는 List를 받아 동적으로 매핑하는 대신, 아래처럼 단순히 이미 매핑되어 있는 Map을 주입받아 사용하면 될 것 같습니다.
@Component
public class CommentAuthorizationService {
private final PostQueryService postQueryService;
private final Map<PostType, CommentAccessPolicy> policyMap;
public CommentAuthorizationService(PostQueryService postQueryService,
Map<PostType, CommentAccessPolicy> policyMap) {
this.postQueryService = postQueryService;
this.policyMap = policyMap;
}
// 나머지 로직은 그대로
}위 부분은 추후에 전략패턴을 적용할 때 많이 언급될 내용인 것 같아서 한번 문서화 해두는 것도 좋을 것 같습니다!!
There was a problem hiding this comment.
전 존재하는 PostType 의 개수가 적으니 지금처럼 supports() 메서드를 사용해서 PostType 별로 Policy 구현체를 찾아서 주입해주는 현재 방식도 괜찮다고 생각했는데, 현준님말처럼 변동될 가능성이 적으니 config 로 명시해주는 것도 좋아보이긴 하네요!!
| private void validateUserCanAccessPostForComment(PostType type, Long postId, Long userId) { | ||
| CommentCountUpdatable post = postQueryService.findPost(type, postId); | ||
| getPolicy(type).validateCommentAccess(post, userId); | ||
| } | ||
|
|
||
| public void validateUserCanAccessPostForComment(PostType type, CommentCountUpdatable post, Long userId) { | ||
| getPolicy(type).validateCommentAccess(post, userId); | ||
| } | ||
|
|
||
| public void validateUserCanAccessPostForComment(Comment comment, Long userId) { | ||
| validateUserCanAccessPostForComment(comment.getPostType(), comment.getTargetPostId(), userId); | ||
| } |
There was a problem hiding this comment.
현재 CommentAuthorizationService에는 세 가지 형태로 validateUserCanAccessPostForComment 메서드가 오버로딩되어 있는 것 같습니다.
그런데 이 과정에서 PostQueryService를 CommentAuthorizationService 내부에서 직접 호출하고 있는데, 이 책임이 이 위치에 있어야 할지에 대해서는 한 번 고민해볼 필요가 있는 것 같습니다.
CommentLikeService나 CommentCreateService에서는 어차피 권한 검증 전에 commentAuthorizationService.validateUserCanAccessPostForComment를 호출하기 위해 postQueryService.findPost()를 항상 거치게 되므로, 조회 책임을 상위 서비스에서 미리 수행하고 CommentAuthorizationService에는 조회된 post 객체를 넘겨 검증만 담당하게 해도 될 것 같습니다.
이렇게 하면 CommentAuthorizationService는 순수하게 도메인 규칙 검증만 담당하게 되고, 현재 구조처럼 Service → DomainService → Service로 이어지는 의존성 방향 문제도 자연스럽게 해소될 수 있을 것 같습니다.
There was a problem hiding this comment.
음 현준님이 도메인 서비스내에서 또 다른 도메인 서비스에 대한 의존성을 가지고 있는 현재 구조때문에 위와 같은 리뷰를 남겨주신것 같은데, 저도 마찬가지로 도메인 서비스가 다른 도메인 서비스를 의존하는 것은 "해당 도메인 서비스가 너무 많은 책임을 가지고 있다" 라고 생각하긴 합니다.
하지만 저는 PostQueryService가 진짜 도메인 서비스라고 할 만 한가? 라는 점을 우선적으로 생각해보고 싶습니다.
package konkuk.thip.common.post.service;
import konkuk.thip.common.post.CommentCountUpdatable;
import konkuk.thip.common.post.PostType;
import konkuk.thip.feed.application.port.out.FeedCommandPort;
import konkuk.thip.record.application.port.out.RecordCommandPort;
import konkuk.thip.vote.application.port.out.VoteCommandPort;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
@Service
@RequiredArgsConstructor
public class PostQueryService {
private final FeedCommandPort feedCommandPort;
private final RecordCommandPort recordCommandPort;
private final VoteCommandPort voteCommandPort;
public CommentCountUpdatable findPost(PostType type, Long postId) {
return switch (type) {
case FEED -> feedCommandPort.getByIdOrThrow(postId);
case RECORD -> recordCommandPort.getByIdOrThrow(postId);
case VOTE -> voteCommandPort.getByIdOrThrow(postId);
};
}
}저는 PostQueryService 가 Post와 관련된 도메인 로직을 구현하기 위해 존재하는 도메인 서비스가 아니라,
"PostType, postId 에 해당하는 도메인을 로드한다" 라는 QueryPort 로의 역할을 수행한다고 생각합니다.
따라서 PostQueryService 에 구현된 메서드를 Port 내부로 옮긴다면,
- CommentAuthorizationService 에서 Port를 의존하는 것으로 수정 & 현재처럼 코드를 전개
- 아니면 현준님 리뷰처럼 아예 어플리케이션 서비스가 도메인을 찾고, 이를 CommentAuthorizationService 로 넘기는 것으로 수정함으로써 CommentAuthorizationService가 Port 의존성도 가지지 않도록 수정
위 2가지 방식 중 어느쪽으로 가도 괜찮지 않나 라고 생각합니다!
There was a problem hiding this comment.
엇 성준님 말씀이 잘 이해가 안되는데, 그렇다면 PostQueryService를 Port로 생각하고 interface로 정의한 후 PostQueryPersistenceAdapter 같은 구현체에서 findPost 메서드에서 하고 있는 분기처리를 수행해야 한다는 말씀인가요??
저도 PostQueryService가 도메인 서비스라고 생각되지 않아, 서비스(CommentCreateService) -> 도메인 서비스(CommentAuthorizationService) -> 서비스(PostQueryService)의 흐름으로 이어지고 있어서 의존성 방향이 어색하다고 생각하고 있습니다.!
또한 CommentLikeService에서 findPost()를 호출하자는 의견을 드린 이유 중 하나는, CommentCreateService에서도 같은 방식으로 유효성 검증을 하고 있기 때문에, 코드의 일관성을 맞추려는 목적도 있었습니다.
There was a problem hiding this comment.
엇 저는 우선 PostQueryService를 도메인 서비스라기보다, 게시물 조회를 담당하는 ‘도메인 로딩 책임’으로 생각하고 작성했습니다.
댓글 생성 시에는 게시물 도메인 객체가 꼭 필요하지만, 좋아요 시는 권한 검증 목적이라 실제 게시물 객체가 꼭 필요하지 않을 수도 있는데, 두 경우가 모두 있으니 유연하게 쓰려고 여러 형태(overload) 메서드를 구현한 거였습니다.
There was a problem hiding this comment.
저도 현준님 말씀처럼 아예 CommentLikeService에서 findPost()를 호출하고자 했는데 댓글 생성과 달리 댓글 좋아요는 게시물 객체가필요하지않은데 이 반환값이 꼭필요한가? 라고 생각되어서 코드가 좀 꼬인것 같긴하네요
그리고 또한 필요하지않은 객체 때문에 CommentLikeService가 postQueryService의존성을 갖는게 의문이 들어서 아예 CommentAuthorizationService가 postQueryService의 의존성을 갖는게 맞지않은가 해서 오버로드하는 형식으로 구현한 것같습니다
그래서 댓글 작성시에는 어차피 게시물 객체가 필요하니 postQueryService,CommentAuthorizationService 의존성을 갖고
댓글 좋아요 시에는 게시물 객체가 필요하지 않으니 CommentAuthorizationService 의존성만 갖자
라는게 구현시의 제 생각이었던 것같습니다
There was a problem hiding this comment.
CommentLikeService나 CommentCreateService에서는 어차피 권한 검증 전에 commentAuthorizationService.validateUserCanAccessPostForComment를 호출하기 위해 postQueryService.findPost()를 항상 거치게 되므로, 조회 책임을 상위 서비스에서 미리 수행하고 CommentAuthorizationService에는 조회된 post 객체를 넘겨 검증만 담당하게 해도 될 것 같습니다.
근데 생각해보니 어차피 댓글 좋아요도 댓글 관련 검증 작업을하려면 게시물 객체가 필요한게 맞으니 아예 상위 서비스에서 게시물 객체를 조회 및 반환하고 CommentAuthorizationService에는 조회된 post 객체를 넘겨 검증만 담당하는것이 둘의 중복 로직이 더 깔끔해지겠네요!! 이렇게 리펙해보도록하겟습니닷 계속 댓글관련만 구현하다보니 머리가 멈췄나보네욬ㅋㅋ
There was a problem hiding this comment.
엇 저는 우선 PostQueryService를 도메인 서비스라기보다, 게시물 조회를 담당하는 ‘도메인 로딩 책임’으로 생각하고 작성했습니다. 댓글 생성 시에는 게시물 도메인 객체가 꼭 필요하지만, 좋아요 시는 권한 검증 목적이라 실제 게시물 객체가 꼭 필요하지 않을 수도 있는데, 두 경우가 모두 있으니 유연하게 쓰려고 여러 형태(overload) 메서드를 구현한 거였습니다.
아하 전 ,,,service 라는 네이밍을 보고 도메인 서비스로 구현하신 걸로 생각했습니다!
PostQueryService 이게 댓글 생성 application service 에서도 사용되고, 댓글 access 검증 도메인 서비스에서도 사용되네요.
구조적인 컨벤션을 무조건 엄밀하게 지켜야한다! 는 아니지만, 뭔가 의존성 방향이 이곳저곳에서 얽힌다는 생각이 들어서 리뷰 달아보았습니다!
저는 분기처리를 하더라도 단순히 도메인 조회해주는 Port 라고 생각하는데, 위에 언급했던것처럼 post 라는 애그리거트가 없으니 어디 정의하기가 애매하네요
There was a problem hiding this comment.
넵 현재 구현된 PostQueryService 의 구현이 Port 에 가깝지 않나 라는 생각에 코멘트 달아보았습니다.
그런데 post 라는 패키지가 없으니 PostQueryPort 라는 친구를 만들기도 애매하고, PostQueryService 의 메서드를 현재 존재하는 FeedQueryPort, RecordQueryPort, VoteQueryPort 중 어디에 넣기도 애매하네요.흠 post 라는 공통 패키지를 만들고, feed, vote, record 를 post 패키지 하위에 몰아야하나? 라는 생각도 들고 애매하네요
음 이부분은 제가 그냥 임의로 댓글관련 구현하면서 post를 관리할일이 많아져서 post 자체가 도메인 패키지와 동일한 선상에 있는것보다는 common -> post 이런식으로 쓰긴했는데 상속구조를 해지해보고 한번 논의가 필요한 부분인것같습니다!!
There was a problem hiding this comment.
아하 전 ,,,service 라는 네이밍을 보고 도메인 서비스로 구현하신 걸로 생각했습니다!
허류 그렇군요 제가 혼동을 드린것같네요.. 도메인서비스는 패키지 구조에서 명확히 domain -> service 패키지를 만들어서 위치했고
PostQueryService는 위에서 말씀드린것처럼 common -> post -> service에 위치해있긴합니닷 (근데 post가 도메인은아니고 그냥 저희 서비스에서 게시물타입?을 정의해둔 느낌으로..)
There was a problem hiding this comment.
아 제가 네이밍을 좀 헷갈리게 하긴했네요 사실상 Port가 맞는것습니다 그럼 정말 성준님 말씀처럼 패키지 위치가 정말 애매해질것같네요
| public interface CommentCountUpdatable { | ||
| void increaseCommentCount(); | ||
| Long getRoomId(); | ||
| } No newline at end of file |
There was a problem hiding this comment.
p2: 현재 CommentCountUpdatable 인터페이스에 getRoomId()가 추가되면서
단순히 댓글 수 관리 책임과 Room 식별자 접근 책임이 한 인터페이스에 같이 묶인 상태인 것 같습니다.
이렇게 되면 Feed처럼 Room과 무관한 도메인도 불필요하게 getRoomId()를 구현해야 하는 문제가 생겨서
역할 분리를 조금 더 명확하게 가져가면 좋을 것 같습니다.
따라서, 댓글 수 관리 책임과 Room 식별 책임 분리를 위해서 Room과 관련된 Post만 두 인터페이스를 모두 구현하는 방식으로 하는 것은 어떨지 제안드려요.
- CommentCountUpdatable: 댓글 수 증감 책임만
- RoomPost : 모임방에 게시되는 게시글(기록, 투표) <- CommentCountUpdatable을 상속
이렇게 하면 Feed는 CommentCountUpdatable만 구현하고,
Record/Vote는 RoomPost를 구현하도록 하여 책임이 명확해질 것 같습니다.
제가 생각하는 예시코드인데 참고 부탁드릴게요!
public interface CommentCountUpdatable {
void increaseCommentCount();
}
/**
* Room과 연관된 Post (Record, Vote)
*/
public interface RoomPost extends CommentCountUpdatable {
Long getRoomId();
}@Component
@RequiredArgsConstructor
public class RoomPostCommentAccessPolicy implements CommentAccessPolicy {
private final RoomParticipantService roomParticipantService;
@Override
public boolean supports(PostType type) {
return type == PostType.RECORD || type == PostType.VOTE;
}
@Override
public void validateCommentAccess(CommentCountUpdatable post, Long userId) {
RoomPost roomPost = (RoomPost) post; // RECORD/VOTE만 들어오므로 안전
roomParticipantService.validateUserIsRoomMember(roomPost.getRoomId(), userId);
}
}There was a problem hiding this comment.
음 사실 현재 RoomPostCommentAccessPolicy에서도 Service의 의존성을 가지는게 맞을까 라는 의문이 들기는 하지만, 더 좋은 방법이 생각나지는 않네요,, 혹시 예리하신 @seongjunnoh님은 어떻게 생각하시나요..?
There was a problem hiding this comment.
CommentCountUpdatable 인터페이스에 null 반환값이 있더라도 getRoomId 메서드가 정의하는 것보다는, 현준님이 언급하신 것처럼 RoomPost 라는 인터페이스를 하나 더 정의하는게 훨씬 좋아보이네요!!
There was a problem hiding this comment.
음 사실 현재 RoomPostCommentAccessPolicy에서도 Service의 의존성을 가지는게 맞을까 라는 의문이 들기는 하지만, 더 좋은 방법이 생각나지는 않네요,, 혹시 예리하신 @seongjunnoh님은 어떻게 생각하시나요..?
음 처음에 제가 제안한 것은 application service 에서 바로 RoomParticipantService 를 호출함으로써 "유저가 해당 방(= 댓글 생성, 댓글 좋아요 달려는)에 속하는지를 검증" 하는 로직을 도메인 서비스의 책임으로 분리하는 구조였는데, 코드 수정 중 "댓글 관련 authorization 을 담당하는 도메인 서비스" 를 추가하시면서 코드가 섞인 것 같네요.
수정된 구조처럼 댓글 관련 authorization을 담당하는 도메인 서비스를 정의하신다면, 저는 RoomPostCommentAccessPolicy 에서 RoomParticipantService 라는 도메인 서비스를 통해 RoomPostComment 에 대한 access validation 을 수행하는 것이 아니라, 그냥 RoomParticipantPort에 의존하여 access validation을 수행하는 것이 맞지 않나 라고 생각합니다
package konkuk.thip.comment.domain.policy;
import konkuk.thip.common.post.CommentCountUpdatable;
import konkuk.thip.common.post.PostType;
import konkuk.thip.room.domain.service.RoomParticipantService;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;
@Component
@RequiredArgsConstructor
public class RoomPostCommentAccessPolicy implements CommentAccessPolicy {
private final RoomParticipantQueryPort participantPort;
@Override
public boolean supports(PostType type) {
return type == PostType.RECORD || type == PostType.VOTE;
}
@Override
public void validateCommentAccess(CommentCountUpdatable post, Long userId) {
if (!participantPort.existByUserIdAndRoomId(post.getRoomId, userId)) {
throw new InvalidStateException(ROOM_ACCESS_FORBIDDEN,
new IllegalArgumentException(",,,"); // comment access 에 fit 한 에러 메시지 작성
}
}
}위와 같이 RoomParticipantService 의 구현을 바로 validateCommentAccess 메서드에 적용하는 구조는 어떤가요??
There was a problem hiding this comment.
엇 근데 저는 우선 기록(Record)나 투표(Vote) 게시물 수정 등 여러 방과 관련된 도메인 로직에서 이미 해당 방 참여 여부를 판단하는 RoomParticipantService가 필수적으로 필요할 것같아서 RoomParticipantService 의존성 해제는 꼭 필요하지 않다고 생각하긴하는데 어떠신가요??
댓글관련접근 정책을 세 게시물 타입으로 합치면서 조금 애매해진것같은데 Record와 Vote 게시물 댓글 관련 접근 정책이 사실상 RoomParticipantService인 느낌이긴 합니다. Feed가 예외적으로 "비공개 글일 때 작성자만 댓글을 쓸 수 있다"라는 별도의 댓글 접근 정책인 것같네요
There was a problem hiding this comment.
음.. 제 생각에는 도메인 서비스는 서비스 클래스나 Port 등 다른 계층의 의존성을 가져서는 안된다고 생각합니다
아무리 서비스 로직이라고 하더라도 도메인 서비스는 도메인 계층에 속하기 때문에 헥사고날 아키텍처의 핵심인 도메인을 순수하게 지키자는 목표에서 벗어나게 된다고 생각해요.
만약 도메인 서비스에서 Port나 Service에 대한 의존성이 필요해지는 상황이라면, 그것은 도메인 서비스가 아니라 또 다른 Usecase 성격의 로직으로 보는 게 맞지 않을까 하는 생각이 드네요. 책에서 읽었던 것처럼 도메인 서비스는 도메인 객체의 상태를 변경하거나 도메인 규칙을 적용하는 순수한 역할에만 집중하는 것이 더 적절하다고 생각합니다.
이 부분 때문에 CommentAuthorizationService가 가지고 있는 PostQueryService의 의존성을 해제하는 것을 제안드린 건데, 현재 RoomPostCommentAccessPolicy 내부에서도 Service 의존성이 존재하기 때문에 제 생각에는 희진님이 작성하신 정책과 권한 검증 관련 클래스들이 모두 사실상 서비스 로직에 가깝지 않나 싶습니다..!
그래서 다음 두 가지 대안을 제안드려요
- 현재 정의된 모든 도메인 서비스를 서비스 계층으로 이동
• 희진님이 작성하신 도메인 서비스 클래스들을 service 패키지 하위로 옮기고, 네이밍도 기존 서비스들과 차별화하기 위해
예: CommentAuthorizationService → CommentAuthorizationValidator, RoomParticipantService → RoomParticipantValidator 같은 식으로 변경하면 어떨까 싶습니다 - 도메인 서비스에서 Port 또는 Service 의존성을 제거
• 외부 의존성을 전부 응용 서비스에서 처리하고, 도메인 서비스에는 이미 조회가 끝난 도메인 객체나 값만 넘겨서 검증만 수행하도록 역할을 분리하면 도메인 계층이 더 깔끔해질 것 같습니다
결국 이 로직들의 핵심 역할은 도메인 규칙을 직접 적용한다기보다는 ‘권한 검증’이라는 유스케이스적 성격에 더 가까워 보이기 때문에, 도메인 계층보다는 서비스 계층으로 위치를 옮기고 책임을 명확히 분리하는 방향이 더 적합하지 않을까 생각합니다
이렇게 하면 응용 서비스에서 데이터 로딩과 정책 선택을 담당하고, 검증 로직은 밸리데이터 성격의 클래스로 분리돼서 서로의 책임이 훨씬 명확해질 것 같아요
There was a problem hiding this comment.
네 현준님 코멘트처럼 Port 의존성을 전부 application service 에서 갖도록 하고, domain service 에서는 그냥 내부로직만 수행하면 domain service 의 코드는 깔끔해질것 같습니다
그런데 저는 domain service가 adapter를 직접적으로 의존하는게 아니라 application 계층의 port 를 의존하는건 괜찮지 않나 생각해서 위와 같은 구조를 제안한 건데, application service 에서 필요한 것들을 모두 찾아오는게 깔끔하긴 하죠
다만 지금 희진님이 구현하신 domain service 에서 담당하는 로직이 영속성 계층으로부터 어떤 도메인들을 찾아와서 로직을 수행하는것이 아니라, 영속성 계층으로부터 "roomId, userId 에 해당하는 RoomParticipant 도메인이 존재하는가" 를 찾아오는 것이므로, Port 의존성 없이 domain service를 어떻게 정의할 수 있을지는 잘 모르곘습니다
따라서 이 경우 위에 현준님이 언급하신 것처럼 저도 1번의 느낌으로 가는게 좋다고 생각합니다!! 네이밍도 Validator 라고 수정하면 더 직관적일 것 같네요!!
다만 이 경우 service -> service 로의 의존성이 생기긴 하겠지만, 이 부분은 전 문제될게 없다고 생각합니다 (Validator 를 헬퍼 서비스로 보고, service 끼리의 의존성 방향을 저희가 잘 유지한다면 문제없다고 생각합니다)
제가 저번주 스터디에서 언급했던 것처럼, 저는 domain service 라는 것은 어떤 유스케이스를 수행하는데 하나의 도메인이 아니라 여러 도메인의 communication 이 필요한 경우를 위해 정의한다고 생각합니다
현재 저희는 use case 1개 당 1개의 applicaiton service를 가지도록 구성하였고, 이때 사실상 여러 도메인을 로드해 도메인들간의 로직을 수행하는 요구사항보다는 일급컬렉션처럼 하나의 도메인에 대한 로직을 수행하고, application service 에서는 필요한 도메인 로직들을 orchestration 하는 것이 일반적이라고 생각합니다
따라서 도메인 서비스를 고려할 때 진짜 여러 도메인들의 도메인 로직이 필요한가? 를 먼저 고민해보는 것이 좋을 것 같다는 생각이 들었습니다! (제가 먼저 도메인 서비스를 제안드렸는데 민망하네요ㅎ)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/konkuk/thip/comment/application/service/policy/FeedCommentAccessPolicy.java (1)
16-21: 캐스팅 안전성 개선을 고려해보세요.직접 캐스팅(
(Feed) post)이 사용되고 있는데, 타입 안전성을 위해instanceof체크를 추가하는 것을 고려해보세요.@Override public void validateCommentAccess(CommentCountUpdatable post, Long userId) { + if (!(post instanceof Feed)) { + throw new InvalidStateException(POST_TYPE_NOT_MATCH); + } Feed feed = (Feed) post; feed.validateCreateComment(userId); }src/main/java/konkuk/thip/comment/application/service/policy/RoomPostCommentAccessPolicy.java (1)
21-25: 캐스팅 안전성 개선을 권장합니다.
FeedCommentAccessPolicy와 마찬가지로 직접 캐스팅이 사용되고 있습니다. 타입 안전성을 위해instanceof체크를 추가해보세요.@Override public void validateCommentAccess(CommentCountUpdatable post, Long userId) { + if (!(post instanceof RoomPost)) { + throw new InvalidStateException(POST_TYPE_NOT_MATCH); + } RoomPost roomPost = (RoomPost) post; roomParticipantValidator.validateUserIsRoomMember(roomPost.getRoomId(), userId); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/konkuk/thip/comment/application/service/CommentAuthorizationValidator.java(1 hunks)src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java(1 hunks)src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java(1 hunks)src/main/java/konkuk/thip/comment/application/service/policy/CommentAccessPolicy.java(1 hunks)src/main/java/konkuk/thip/comment/application/service/policy/FeedCommentAccessPolicy.java(1 hunks)src/main/java/konkuk/thip/comment/application/service/policy/RoomPostCommentAccessPolicy.java(1 hunks)src/main/java/konkuk/thip/common/post/CommentCountUpdatable.java(1 hunks)src/main/java/konkuk/thip/config/CommentAccessPolicyConfig.java(1 hunks)src/main/java/konkuk/thip/feed/domain/Feed.java(4 hunks)src/main/java/konkuk/thip/record/domain/Record.java(3 hunks)src/main/java/konkuk/thip/room/application/service/RoomParticipantValidator.java(1 hunks)src/main/java/konkuk/thip/room/domain/RoomPost.java(1 hunks)src/main/java/konkuk/thip/vote/domain/Vote.java(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/java/konkuk/thip/comment/application/service/policy/CommentAccessPolicy.java
- src/main/java/konkuk/thip/room/domain/RoomPost.java
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/konkuk/thip/common/post/CommentCountUpdatable.java
- src/main/java/konkuk/thip/record/domain/Record.java
- src/main/java/konkuk/thip/vote/domain/Vote.java
- src/main/java/konkuk/thip/feed/domain/Feed.java
- src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java
🧰 Additional context used
🧠 Learnings (3)
src/main/java/konkuk/thip/room/application/service/RoomParticipantValidator.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 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
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의 참가자인지 검증이 필요하다.
src/main/java/konkuk/thip/comment/application/service/policy/RoomPostCommentAccessPolicy.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 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
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의 참가자인지 검증이 필요하다.
src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java (3)
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 등에서도 함께 사용될 가능성이 높습니다.
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: seongjunnoh
PR: #43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
⏰ 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 (13)
src/main/java/konkuk/thip/comment/application/service/policy/FeedCommentAccessPolicy.java (1)
8-14: 정책 패턴 구현이 깔끔합니다.
CommentAccessPolicy인터페이스를 구현하여 피드 댓글 접근 권한을 캡슐화한 구조가 좋습니다.supports()메서드로PostType.FEED에 대한 지원을 명확히 선언했습니다.src/main/java/konkuk/thip/room/application/service/RoomParticipantValidator.java (1)
10-21: 방 참가자 검증 로직이 잘 캡슐화되었습니다.이전 학습 내용에 따라 방 참가자 검증 로직을 별도 서비스로 분리하여 재사용성을 높인 설계가 훌륭합니다. 적절한 예외 처리와 에러 코드(
ROOM_ACCESS_FORBIDDEN) 사용도 좋습니다.src/main/java/konkuk/thip/comment/application/service/CommentAuthorizationValidator.java (2)
14-22: 중앙집중식 인증 검증 설계가 우수합니다.정책 맵을 활용한 위임 패턴으로 각 포스트 타입별 인증 로직을 깔끔하게 분리했습니다.
validateUserCanAccessPostForComment메서드가 일관된 인터페이스를 제공하여 서비스 레이어에서 쉽게 사용할 수 있겠습니다.
24-30: 방어적 프로그래밍이 잘 적용되었습니다.정책을 찾지 못한 경우 적절한 예외(
POST_TYPE_NOT_MATCH)를 던지는 방어적 코드가 좋습니다. null 체크와 예외 처리가 견고합니다.src/main/java/konkuk/thip/config/CommentAccessPolicyConfig.java (2)
13-18: 의존성 주입과 설정 구조가 깔끔합니다.
@Configuration과@RequiredArgsConstructor를 활용한 설정 클래스 구조가 Spring 모범 사례를 잘 따르고 있습니다.
20-27: 불변 맵 사용이 좋은 설계입니다.
Map.of()를 사용하여 불변 맵을 생성한 것이 설정의 안정성을 보장합니다.RECORD와VOTE가 모두 방 기반 포스트이므로 같은 정책(roomCommentPolicy)을 공유하는 것이 논리적으로 적절합니다.src/main/java/konkuk/thip/comment/application/service/policy/RoomPostCommentAccessPolicy.java (1)
10-19: 방 기반 포스트 정책 구현이 적절합니다.
RoomParticipantValidator를 의존성으로 주입받아 방 멤버십 검증을 위임하는 구조가 이전 학습 내용과 일치하며,RECORD와VOTE타입을 모두 지원하는 것이 도메인 요구사항에 맞습니다.src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java (6)
16-25: 클래스 구조와 의존성이 적절하게 설계되었습니다.Service 계층의 책임 분리가 잘 되어 있고, CQRS 패턴에 따라 Command/Query Port를 적절히 분리하여 사용하고 있습니다. PostQueryService와 CommentAuthorizationValidator를 통한 횡단 관심사 처리도 적절합니다.
28-29: 트랜잭션 처리가 적절합니다.댓글 좋아요 상태 변경과 댓글 업데이트가 하나의 트랜잭션으로 묶여 데이터 일관성이 보장됩니다.
32-35: 권한 검증 로직이 개선되었습니다.이전 리뷰에서 언급된 방 참여자 검증 로직이 CommentAuthorizationValidator를 통해 적절히 구현되었습니다. PostQueryService를 통해 게시글을 조회하고, 게시글 타입에 따른 권한 검증이 수행됩니다.
37-38: 성능 최적화가 적절히 반영되었습니다.이전 리뷰 코멘트에서 제안된
isLikedCommentByUser메서드를 사용하여 전체 좋아요 목록을 조회하지 않고 특정 댓글의 좋아요 상태만 확인하도록 개선되었습니다. 이는 성능 측면에서 훨씬 효율적입니다.
40-47: 비즈니스 로직 검증이 도메인 레벨에서 적절히 처리되었습니다.좋아요/좋아요 취소 가능 여부를 Comment 도메인에서 검증하고, 실제 저장/삭제 작업을 Command Port를 통해 수행하는 구조가 깔끔합니다. 도메인 중심 설계 원칙을 잘 따르고 있습니다.
49-54: 댓글 업데이트와 결과 반환이 적절합니다.좋아요 수 업데이트를 도메인 메서드를 통해 수행하고, 최종 결과를 명시적으로 반환하는 구조가 좋습니다. 메서드의 응답성과 일관성을 보장합니다.
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
피드백 모두 잘 반영해서 수정해주신 것 같네요!
앞으로 CommentAuthorizationValidator, Policy 등의 헬퍼 서비스가 use case에 대한 의존성을 가지지 않도록 신경쓰고, 헬퍼 서비스에 도메인 관련 로직이 들어가지 않도록 한다면 현재 구조는 문제 없을 것 같습니다!!
헬퍼 서비스가 별도의 트랜잭션을 가지지 않으니 트랜잭션 관리 또한 최상위 application service의 메서드가 담당하니 트랜잭션이 꼬일 경우도 없을 것 같네요!! (다만 성능적인 이슈는 리펙단계에서 논의해봐야 할 것 같습니다)
고생하셨습니다!!
| import konkuk.thip.common.post.PostType; | ||
|
|
||
| public interface CommentAccessPolicy { | ||
| boolean supports(PostType type); |
There was a problem hiding this comment.
이 메서드는 이제 삭제하셔도 될 듯 합니다!
hd0rable
dismissed
seongjunnoh’s stale review
The merge-base changed after approval.
| @Component | ||
| @RequiredArgsConstructor | ||
| public class CommentAuthorizationValidator { | ||
|
|
||
| private final Map<PostType, CommentAccessPolicy> policyMap; | ||
|
|
||
| public void validateUserCanAccessPostForComment(PostType type, CommentCountUpdatable post, Long userId) { | ||
| getPolicy(type).validateCommentAccess(post, userId); | ||
| } | ||
|
|
||
| private CommentAccessPolicy getPolicy(PostType type) { | ||
| CommentAccessPolicy policy = policyMap.get(type); | ||
| if (policy == null) { | ||
| throw new InvalidStateException(POST_TYPE_NOT_MATCH); | ||
| } | ||
| return policy; | ||
| } | ||
| } |
| public interface RoomPost extends CommentCountUpdatable { | ||
| Long getRoomId(); | ||
| } No newline at end of file |
| @@ -1,4 +1,4 @@ | |||
| package konkuk.thip.room.domain.service; | |||
| package konkuk.thip.room.application.service; | |||
There was a problem hiding this comment.
RoomParticipantValidator도 Usecase의 구현체가 아닌 헬퍼 서비스 느낌이니까 service.validator 패키지 하위에 넣는 것 어떤가요?
hd0rable
dismissed
buzz0331’s stale review
The merge-base changed after approval.
hd0rable
dismissed
buzz0331’s stale review
The merge-base changed after approval.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java(1 hunks)src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java(1 hunks)src/main/java/konkuk/thip/comment/application/service/policy/RoomPostCommentAccessPolicy.java(1 hunks)src/main/java/konkuk/thip/comment/application/service/validator/CommentAuthorizationValidator.java(1 hunks)src/main/java/konkuk/thip/room/application/service/validator/RoomParticipantValidator.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/konkuk/thip/comment/application/service/policy/RoomPostCommentAccessPolicy.java
- src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java
- src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java
🧰 Additional context used
🧠 Learnings (2)
📓 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)에서 다룬다.
src/main/java/konkuk/thip/room/application/service/validator/RoomParticipantValidator.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 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
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의 참가자인지 검증이 필요하다.
🧬 Code Graph Analysis (1)
src/main/java/konkuk/thip/room/application/service/validator/RoomParticipantValidator.java (2)
src/main/java/konkuk/thip/comment/application/service/validator/CommentAuthorizationValidator.java (1)
Component(14-31)src/main/java/konkuk/thip/comment/application/service/policy/RoomPostCommentAccessPolicy.java (1)
Component(9-21)
⏰ 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 (2)
src/main/java/konkuk/thip/comment/application/service/validator/CommentAuthorizationValidator.java (2)
14-31: 전략 패턴을 활용한 우수한 구현입니다.댓글 권한 검증 로직을 중앙화하고 전략 패턴을 통해 PostType별로 다른 정책을 적용할 수 있도록 깔끔하게 구현되었습니다. Map을 활용한 정책 조회와 적절한 예외 처리도 잘 되어 있습니다.
장점:
- 전략 패턴으로 PostType별 권한 정책을 분리하여 확장성 확보
- 중앙화된 검증 로직으로 코드 중복 방지
- null 체크와 명확한 예외 처리로 안정성 보장
20-22: 메서드명과 구현이 일치하며 명확합니다.
validateUserCanAccessPostForComment메서드는 이름이 목적을 명확히 표현하며, 정책으로의 위임을 통해 단일 책임 원칙을 잘 준수하고 있습니다.
| @Component | ||
| @RequiredArgsConstructor | ||
| public class RoomParticipantValidator{ | ||
| private final RoomParticipantQueryPort participantPort; | ||
|
|
||
| // 사용자가 방에 속해있는지 검증 | ||
| public void validateUserIsRoomMember(Long roomId, Long userId) { | ||
| if (!participantPort.existByUserIdAndRoomId(roomId, userId)) { | ||
| throw new InvalidStateException(ROOM_ACCESS_FORBIDDEN, | ||
| new IllegalArgumentException("사용자가 이 방의 참가자가 아닙니다. roomId=" + roomId + ", userId=" + userId)); | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
도메인 서비스 패턴으로 리팩토링을 고려해보세요.
현재 구현은 정상적으로 작동하지만, 검색된 학습 내용에 따르면 THIP 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이는 방식을 선호합니다.
현재 application 계층의 validator보다는 domain 계층의 policy 서비스로 구현하는 것이 비즈니스 로직의 중복을 방지하고 재사용성을 높일 수 있습니다.
도메인 서비스 패턴으로 리팩토링:
-package konkuk.thip.room.application.service.validator;
+package konkuk.thip.room.domain.service;
-@Component
-@RequiredArgsConstructor
-public class RoomParticipantValidator{
+@Component
+@RequiredArgsConstructor
+public class RoomParticipantPolicy {
private final RoomParticipantQueryPort participantPort;
- // 사용자가 방에 속해있는지 검증
- public void validateUserIsRoomMember(Long roomId, Long userId) {
+ public void validateUserIsRoomMember(Long roomId, Long userId) {
if (!participantPort.existByUserIdAndRoomId(roomId, userId)) {
throw new InvalidStateException(ROOM_ACCESS_FORBIDDEN,
new IllegalArgumentException("사용자가 이 방의 참가자가 아닙니다. roomId=" + roomId + ", userId=" + userId));
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Component | |
| @RequiredArgsConstructor | |
| public class RoomParticipantValidator{ | |
| private final RoomParticipantQueryPort participantPort; | |
| // 사용자가 방에 속해있는지 검증 | |
| public void validateUserIsRoomMember(Long roomId, Long userId) { | |
| if (!participantPort.existByUserIdAndRoomId(roomId, userId)) { | |
| throw new InvalidStateException(ROOM_ACCESS_FORBIDDEN, | |
| new IllegalArgumentException("사용자가 이 방의 참가자가 아닙니다. roomId=" + roomId + ", userId=" + userId)); | |
| } | |
| } | |
| } | |
| package konkuk.thip.room.domain.service; | |
| import org.springframework.stereotype.Component; | |
| import lombok.RequiredArgsConstructor; | |
| @Component | |
| @RequiredArgsConstructor | |
| public class RoomParticipantPolicy { | |
| private final RoomParticipantQueryPort participantPort; | |
| public void validateUserIsRoomMember(Long roomId, Long userId) { | |
| if (!participantPort.existByUserIdAndRoomId(roomId, userId)) { | |
| throw new InvalidStateException(ROOM_ACCESS_FORBIDDEN, | |
| new IllegalArgumentException("사용자가 이 방의 참가자가 아닙니다. roomId=" + roomId + ", userId=" + userId)); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/room/application/service/validator/RoomParticipantValidator.java
around lines 10 to 22, refactor the user membership validation logic from the
application layer validator to a domain service named RoomParticipantPolicy.
Move the method that checks if a user belongs to a room into this domain service
to encapsulate business rules, improve reusability, and avoid duplication. Then
update the validator or calling code to use this domain service instead of
directly querying the participant port.
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
버그 수정
테스트
기타