Conversation
|
""" Walkthrough이번 변경 사항은 방 참가 및 참가 취소 기능을 위한 API, 서비스, 도메인, 포트, 퍼시스턴스, 예외 코드, 테스트 등을 전반적으로 추가 및 개선한 내용입니다. 참가자 저장/삭제, 멤버 수 관리, 참가 상태 검증, soft delete 적용, 쿼리 리포지토리 분리, 그리고 관련 통합 및 단위 테스트가 포함되어 있습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant RoomJoinService
participant RoomCommandPort
participant RoomParticipantCommandPort
participant RoomParticipantQueryPort
Client->>Controller: POST /rooms/{roomId}/join (RoomJoinRequest)
Controller->>RoomJoinService: changeJoinState(RoomJoinCommand)
RoomJoinService->>RoomCommandPort: 방 조회 (roomId)
RoomJoinService->>RoomParticipantQueryPort: 참가자 존재 여부 확인
alt Join 요청
RoomJoinService->>RoomParticipantCommandPort: 참가자 저장
RoomJoinService->>RoomCommandPort: 멤버 수 증가/업데이트
else Cancel 요청
RoomJoinService->>RoomParticipantCommandPort: 참가자 삭제
RoomJoinService->>RoomCommandPort: 멤버 수 감소/업데이트
end
RoomJoinService-->>Controller: void
Controller-->>Client: 성공 응답
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (10)
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantQueryRepository.java (1)
3-4: 빈 인터페이스에 대한 설명이 필요합니다.현재 빈 인터페이스로 되어 있는데, 향후 커스텀 쿼리 메서드를 위한 인터페이스라면 주석으로 목적을 명시하는 것이 좋겠습니다.
+/** + * RoomParticipant 관련 커스텀 쿼리 메서드를 위한 인터페이스 + */ public interface RoomParticipantQueryRepository { }src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java (1)
59-61: 멤버 카운트 업데이트 메서드가 적절합니다.도메인 레이어에서 검증 로직을 처리하고 이 메서드는 단순 데이터 저장 역할을 하는 것이 맞습니다. 다만 매개변수 검증을 위해 간단한 주석을 추가하는 것을 권장합니다.
+ /** + * 멤버 카운트를 업데이트합니다. + * 주의: 도메인 레이어에서 검증된 값만 전달받아야 합니다. + */ public void updateMemberCount(int count) { this.memberCount = count; }src/main/java/konkuk/thip/room/adapter/in/web/request/RoomJoinRequest.java (1)
6-9: 검증 메시지의 오타 수정 필요record 기반 DTO 설계가 좋습니다. 하지만 validation 메시지에 오타가 있습니다.
다음과 같이 수정하세요:
- @NotBlank(message = "방 참여 유형 파라미터는 필수입니다..") + @NotBlank(message = "방 참여 유형 파라미터는 필수입니다.")src/main/java/konkuk/thip/room/domain/RoomJoinType.java (1)
18-25: 성능 개선을 위한 더 효율적인 구현 고려현재 구현은 정확하지만, 단순한 2개 값의 enum에 대해 stream API를 사용하는 것은 다소 무겁습니다. 더 효율적인 구현을 제안합니다.
- public static RoomJoinType from(String type) { - return Arrays.stream(RoomJoinType.values()) - .filter(param -> param.getType().equals(type)) - .findFirst() - .orElseThrow( - () -> new InvalidStateException(ErrorCode.ROOM_JOIN_TYPE_NOT_MATCH) - ); - } + public static RoomJoinType from(String type) { + return switch (type) { + case "join" -> JOIN; + case "cancel" -> CANCEL; + default -> throw new InvalidStateException(ErrorCode.ROOM_JOIN_TYPE_NOT_MATCH); + }; + }src/main/java/konkuk/thip/room/adapter/in/web/RoomCommandController.java (1)
38-40: 매개변수 일관성 개선 제안메서드 매개변수에
final키워드를 일관되게 사용하고 있지만, 기존createRoom메서드와의 일관성을 위해 동일한 스타일을 적용하는 것이 좋겠습니다.- public BaseResponse joinRoom(@Valid @RequestBody final RoomJoinRequest request, - @UserId final Long userId, - @PathVariable final Long roomId) { + public BaseResponse joinRoom(@Valid @RequestBody RoomJoinRequest request, + @UserId Long userId, + @PathVariable Long roomId) {src/main/java/konkuk/thip/room/domain/RoomParticipant.java (1)
55-55: 문자열 비교 대신 enum 비교 고려현재
roomParticipantRole필드가String타입이므로 문자열 비교를 사용하고 있지만, 향후 enum 타입으로 리팩토링을 고려해보시기 바랍니다.현재 구현은 정확하지만, 타입 안전성을 위해 향후 다음과 같은 개선을 고려할 수 있습니다:
// 만약 roomParticipantRole이 enum으로 변경된다면 if (this.roomParticipantRole == RoomParticipantRole.HOST) { throw new BusinessException(ErrorCode.HOST_CANNOT_CANCEL); }src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java (1)
13-14: Spring Data JPA 쿼리 메서드 사용을 고려해보세요Native 쿼리 대신 Spring Data JPA의 쿼리 메서드를 사용하면 더 간결하고 유지보수가 용이합니다:
-@Query(value = "SELECT * FROM room_participants WHERE user_id = :userId AND room_id = :roomId", nativeQuery = true) -Optional<RoomParticipantJpaEntity> findByUserIdAndRoomId(@Param("userId") Long userId, @Param("roomId") Long roomId); +Optional<RoomParticipantJpaEntity> findByUserIdAndRoomId(Long userId, Long roomId);만약 엔티티의 필드명이
userId,roomId라면 Spring Data JPA가 자동으로 쿼리를 생성합니다.src/main/java/konkuk/thip/room/application/service/RoomJoinService.java (1)
59-59: 가독성 개선을 위한 메서드 추가 제안부정 연산자 대신 명시적인 메서드를 사용하면 가독성이 향상됩니다:
-if(!type.isJoinType()) { +if(type.isCancelType()) {
RoomJoinTypeenum에isCancelType()메서드를 추가하는 것을 고려해보세요.src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (2)
85-86: 에러 메시지 표현을 조금 더 명확하게 다듬어 보시는 것은 어떨까요?
ROOM_MEMBER_COUNT_UNDERFLOW의 현재 메시지"방의 인원 수가 1 이하(방장 포함)입니다."는 조건은 잘 설명하지만 “어떤 행위가 제한된다”는 뉘앙스가 부족해 보입니다.
예를 들어 참여 취소 시 최소 인원 제약 때문에 실패한다는 의미라면 아래처럼 약간만 보강해 가독성을 높여볼 수 있습니다.- ROOM_MEMBER_COUNT_UNDERFLOW(HttpStatus.BAD_REQUEST, 100007, "방의 인원 수가 1 이하(방장 포함)입니다."), + ROOM_MEMBER_COUNT_UNDERFLOW(HttpStatus.BAD_REQUEST, 100007, "방장 포함 인원이 1명 이하인 방은 참여 취소가 불가합니다."),
114-118: HTTP 상태·코드 메시지 정합성 및 네이밍 개선 제안
USER_CANNOT_JOIN_OR_CANCEL은 “존재하지 않는 방” 상황을 나타내며, 이미ROOM_NOT_FOUND(404) 코드가 있습니다. 의미가 겹치므로
- 코드 자체를 제거하고
ROOM_NOT_FOUND로 통합- 혹은 메시지를 “모집기간이 종료된 방은 참여/취소가 불가합니다”와 같이 별도 비즈니스 케이스로 명확히 구분하는 방안을 권장드립니다.
상수 네이밍과 문장형 메시지는 일관성을 유지하면 유지·검색에 도움이 됩니다.
USER_ALREADY_PARTICIPATE→USER_ALREADY_PARTICIPATED(과거분사 일관성)HOST_CANNOT_CANCEL메시지"방장은 참여 취소를 할 수 없습니다."는 명확하지만, 동일 패턴인 “~할 수 없습니다.” 형태로 앞선 메시지들과 통일할 수 있습니다.예시 diff:
- USER_ALREADY_PARTICIPATE(HttpStatus.BAD_REQUEST, 140005, "사용자가 이미 방에 참여한 상태입니다."), + USER_ALREADY_PARTICIPATED(HttpStatus.BAD_REQUEST, 140005, "사용자가 이미 방에 참여한 상태입니다."),필요 시 서비스·컨트롤러 레이어 레퍼런스도 함께 변경해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(2 hunks)src/main/java/konkuk/thip/room/adapter/in/web/RoomCommandController.java(2 hunks)src/main/java/konkuk/thip/room/adapter/in/web/request/RoomJoinRequest.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantRole.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantCommandPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantQueryRepository.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantQueryRepositoryImpl.java(1 hunks)src/main/java/konkuk/thip/room/application/port/in/RoomJoinUsecase.java(1 hunks)src/main/java/konkuk/thip/room/application/port/in/dto/RoomJoinCommand.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomParticipantCommandPort.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomParticipantQueryPort.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomJoinService.java(1 hunks)src/main/java/konkuk/thip/room/domain/Room.java(2 hunks)src/main/java/konkuk/thip/room/domain/RoomJoinType.java(1 hunks)src/main/java/konkuk/thip/room/domain/RoomParticipant.java(3 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/repository/following/FollowingQueryRepository.java(0 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/repository/following/FollowingQueryRepositoryImpl.java(0 hunks)src/test/java/konkuk/thip/common/util/TestEntityFactory.java(2 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateAPITest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java(1 hunks)src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java(1 hunks)src/test/java/konkuk/thip/room/domain/RoomTest.java(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/konkuk/thip/user/adapter/out/persistence/repository/following/FollowingQueryRepositoryImpl.java
- src/main/java/konkuk/thip/user/adapter/out/persistence/repository/following/FollowingQueryRepository.java
🧰 Additional context used
🧠 Learnings (9)
src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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/test/java/konkuk/thip/room/adapter/in/web/RoomCreateAPITest.java (1)
Learnt from: hd0rable
PR: THIP-TextHip/THIP-Server#57
File: src/test/java/konkuk/thip/room/domain/RoomTest.java:0-0
Timestamp: 2025-07-08T16:30:33.771Z
Learning: Room 도메인에서 startDate는 현재 날짜 이후여야 하는 도메인 규칙이 있어서, 테스트에서 만료된 상태를 시뮬레이션하려면 reflection을 사용해야 한다.
src/main/java/konkuk/thip/room/application/port/out/RoomParticipantQueryPort.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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/application/port/out/RoomParticipantCommandPort.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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/adapter/out/persistence/RoomParticipantQueryPersistenceAdapter.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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/Room.java (1)
Learnt from: hd0rable
PR: THIP-TextHip/THIP-Server#57
File: src/test/java/konkuk/thip/room/domain/RoomTest.java:0-0
Timestamp: 2025-07-08T16:30:33.771Z
Learning: Room 도메인에서 startDate는 현재 날짜 이후여야 하는 도메인 규칙이 있어서, 테스트에서 만료된 상태를 시뮬레이션하려면 reflection을 사용해야 한다.
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (1)
Learnt from: hd0rable
PR: THIP-TextHip/THIP-Server#57
File: src/test/java/konkuk/thip/room/domain/RoomTest.java:0-0
Timestamp: 2025-07-08T16:30:33.771Z
Learning: Room 도메인에서 startDate는 현재 날짜 이후여야 하는 도메인 규칙이 있어서, 테스트에서 만료된 상태를 시뮬레이션하려면 reflection을 사용해야 한다.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantCommandPersistenceAdapter.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java (2)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
🧬 Code Graph Analysis (2)
src/main/java/konkuk/thip/room/domain/RoomParticipant.java (1)
src/main/java/konkuk/thip/room/domain/RoomParticipants.java (1)
Getter(12-55)
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(21-172)
🪛 GitHub Actions: CI with Gradle
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java
[error] 181-181: Compilation error: incompatible types: Long cannot be converted to boolean at line 181.
🔇 Additional comments (28)
src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateAPITest.java (1)
42-42: 테스트 명명 규칙 개선이 좋습니다.통합 테스트임을 명확히 하는 "[통합]" 접두사 추가로 테스트 분류가 더 명확해졌습니다.
src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java (1)
14-14: CQRS 패턴을 잘 따른 메서드 추가입니다.도메인 엔티티를 매개변수로 받는 CommandPort의 기존 컨벤션을 잘 따랐습니다.
src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantRole.java (1)
14-14: 불변성 개선이 좋습니다.enum 필드를 final로 만든 것은 불변성을 보장하고 스레드 안전성을 향상시키는 좋은 개선입니다.
src/main/java/konkuk/thip/room/application/port/out/RoomParticipantQueryPort.java (1)
4-6: 인터페이스 설계가 적절합니다.참가자 존재 여부를 확인하는 목적에 맞는 간단하고 명확한 인터페이스입니다. 메서드명이 Spring Data JPA 컨벤션을 따르고 있어 직관적입니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomCommandPersistenceAdapter.java (1)
58-67: 멤버 카운트 업데이트 구현이 적절합니다.기존 코드와 일관된 패턴을 따르고 있으며, 엔티티가 존재하지 않을 경우 적절한 예외 처리를 하고 있습니다. 도메인 객체의 상태를 JPA 엔티티에 반영하는 로직이 명확합니다.
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (2)
4-5: Import 정리가 좋습니다.와일드카드 import를 명시적 import로 변경하여 의존성을 명확하게 했습니다. 코드 가독성과 유지보수성이 향상됩니다.
Also applies to: 12-16
19-19: 테스트 데이터 품질 개선이 우수합니다.하드코딩된 ISBN 대신 UUID 기반으로 동적 생성하여 테스트 간 충돌을 방지했습니다. 13자리 제한도 적절하게 적용되어 있습니다.
Also applies to: 73-73
src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.java (1)
7-7: import 문 추가 승인Hibernate의
@SQLDelete어노테이션을 사용하기 위한 import 문이 올바르게 추가되었습니다.src/main/java/konkuk/thip/room/application/port/in/RoomJoinUsecase.java (1)
1-9: 깔끔한 유스케이스 인터페이스 설계단일 책임 원칙을 잘 따르는 깔끔한 인터페이스입니다.
changeJoinState메서드명이 참여/취소 둘 다를 처리한다는 의미를 잘 표현하고 있으며, CQRS 패턴의 command 작업에 적합한 void 반환 타입을 사용했습니다.src/main/java/konkuk/thip/room/application/port/out/RoomParticipantCommandPort.java (1)
12-14: CQRS 컨벤션에 맞는 메서드 추가
save와deleteByUserIdAndRoomId메서드가 CommandPort에 올바르게 추가되었습니다. 프로젝트의 CQRS 포트 분리 컨벤션을 잘 따르고 있으며, command 작업에 적합한 void 반환 타입을 사용했습니다.src/main/java/konkuk/thip/room/adapter/in/web/request/RoomJoinRequest.java (1)
10-12: 도메인 커맨드 변환 메서드 잘 구현됨
toCommand메서드가 Request DTO를 도메인 커맨드로 변환하는 역할을 명확하게 수행하고 있습니다. 계층 간 데이터 변환 패턴을 잘 따르고 있습니다.src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantQueryPersistenceAdapter.java (2)
8-10: 어댑터 클래스 구조가 올바름
@Repository어노테이션과@RequiredArgsConstructor를 적절히 사용하여 Spring의 의존성 주입을 활용한 깔끔한 어댑터 구조입니다.
14-17: 존재 여부 확인 로직이 올바름repository에서 반환하는 count 값을 1과 비교하여 boolean으로 변환하는 로직이 올바르게 구현되었습니다. 존재 여부 확인에 적합한 구현입니다.
src/main/java/konkuk/thip/room/domain/RoomJoinType.java (1)
27-29: 명확하고 직관적인 메서드 구현
isJoinType()메서드 구현이 명확하고 직관적입니다.src/main/java/konkuk/thip/room/adapter/in/web/RoomCommandController.java (1)
37-44: 새로운 참여/취소 API 엔드포인트 구현이 적절함새로운 엔드포인트의 구현이 기존 패턴과 일관성을 유지하고 있으며, 적절한 검증 애노테이션과 관심사의 분리가 잘 되어 있습니다.
src/test/java/konkuk/thip/room/domain/RoomTest.java (2)
243-253: 경계 조건 테스트가 적절함
increaseMemberCount의 정원 초과 상황에 대한 테스트가 적절하게 구현되어 있습니다. 특정 에러 코드 검증도 포함되어 있어 API 계약을 명확히 하고 있습니다.
255-265: 멤버 카운트 언더플로우 테스트가 적절함방장만 남은 상황에서
decreaseMemberCount호출 시 예외 발생을 검증하는 테스트가 잘 구현되어 있습니다. 도메인 규칙을 명확히 검증하고 있습니다.src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (2)
122-144: 통합 테스트 구현이 포괄적임방 참여 성공 케이스에 대한 테스트가 잘 구현되어 있습니다. API 응답과 데이터베이스 상태 변경을 모두 검증하고 있어 통합 테스트로서 적절합니다.
59-67: 테스트 데이터 설정 메서드가 명확함
setUpWithOnlyHost메서드가 명확하고 재사용 가능한 형태로 구현되어 있습니다. 테스트 시나리오별로 적절한 데이터 설정이 이루어지고 있습니다.src/main/java/konkuk/thip/room/domain/RoomParticipant.java (2)
28-36: 정적 팩토리 메서드 구현이 적절함
withoutId정적 팩토리 메서드가 적절한 기본값들을 설정하고 있으며, 도메인 객체 생성 패턴을 일관되게 따르고 있습니다.
53-58: 비즈니스 규칙 검증 로직이 명확함방장의 참여 취소를 방지하는 검증 로직이 명확하게 구현되어 있습니다. 적절한 예외 처리와 에러 코드 사용이 이루어지고 있습니다.
src/main/java/konkuk/thip/room/domain/Room.java (3)
122-122: 리팩토링이 적절합니다!모집 기간 만료 검증 로직을 별도 메서드로 추출하여 재사용성을 높인 점이 좋습니다.
Also applies to: 135-144
152-155: 멤버 수 증가 로직이 적절합니다!모집 인원 초과를 사전에 검증하는 로직이 안전하게 구현되었습니다.
Also applies to: 162-166
157-160: 멤버 수 감소 로직이 안전합니다!방장을 포함하여 최소 1명이 유지되도록 하는 도메인 제약이 잘 구현되었습니다.
Also applies to: 168-172
src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java (1)
23-118: 테스트가 잘 구성되었습니다!참여/취소 시나리오에 대한 단위 테스트가 체계적으로 작성되었습니다. 특히 예외 케이스들을 꼼꼼하게 검증하는 점이 좋습니다.
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java (1)
29-30: 트랜잭션 처리가 적절합니다참여/취소 로직의 원자성을 보장하는 트랜잭션 경계 설정이 올바르게 되어있습니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantCommandPersistenceAdapter.java (2)
48-60: 저장 로직이 안전하게 구현되었습니다사용자와 방의 존재 여부를 사전에 검증하여 데이터 무결성을 보장하는 점이 좋습니다.
34-34: 리포지토리 메서드 네이밍이 개선되었습니다복잡한 경로 표현식 대신 간단한 메서드명을 사용하여 가독성이 향상되었습니다.
| public record RoomJoinCommand( | ||
| Long userId, | ||
| Long roomId, | ||
| String type | ||
| ) { | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
타입 안전성을 위해 enum 사용을 고려해보세요.
type 필드가 String으로 되어 있는데, "join"과 "cancel" 같은 고정된 값들을 다루는 것 같습니다. 런타임 에러를 방지하기 위해 enum을 사용하는 것이 좋겠습니다.
다음과 같이 enum을 도입하는 것을 제안합니다:
+public enum RoomJoinType {
+ JOIN, CANCEL
+}
+
public record RoomJoinCommand(
Long userId,
Long roomId,
- String type
+ RoomJoinType type
) {
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/room/application/port/in/dto/RoomJoinCommand.java
lines 3 to 8, the type field is currently a String representing fixed values
like "join" and "cancel". To improve type safety and prevent runtime errors,
define an enum with these fixed values and replace the String type field with
this enum type in the RoomJoinCommand record.
| package konkuk.thip.room.adapter.out.persistence.repository.roomparticipant; | ||
|
|
||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| @Repository | ||
| @RequiredArgsConstructor | ||
| public class RoomParticipantQueryRepositoryImpl implements RoomParticipantQueryRepository{ | ||
|
|
||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
빈 구현체 클래스의 필요성을 확인해주세요.
현재 RoomParticipantQueryRepositoryImpl가 비어있는 구현체입니다. 커스텀 쿼리 메서드를 추가할 계획이 없다면 불필요한 클래스일 수 있습니다.
다음 스크립트로 이 인터페이스가 어떻게 사용되는지 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# RoomParticipantQueryRepository 인터페이스 사용 패턴 확인
ast-grep --pattern $'interface RoomParticipantQueryRepository {
$$$
}'
# 구현체에서 실제로 구현해야 할 메서드가 있는지 확인
rg -A 5 "RoomParticipantQueryRepository"Length of output: 2020
RoomParticipantQueryRepositoryImpl 빈 구현체 제거 또는 커스텀 쿼리 정의 필요
RoomParticipantQueryRepository 인터페이스가 메서드를 하나도 선언하지 않아, 현재 빈 구현체인 RoomParticipantQueryRepositoryImpl는 실제로 사용되지 않습니다.
커스텀 쿼리를 구현할 계획이 없다면 해당 인터페이스와 구현체를 제거하고, RoomParticipantJpaRepository만 사용하도록 정리해주세요.
만약 향후 커스텀 쿼리가 필요하다면, 인터페이스에 메서드를 선언한 뒤 구현체에 로직을 추가하는 방향으로 유지하시면 됩니다.
확인 및 조치가 필요한 위치:
- src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantQueryRepository.java
- src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantQueryRepositoryImpl.java
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantQueryRepositoryImpl.java
and RoomParticipantQueryRepository.java at the start of the files, the
RoomParticipantQueryRepository interface declares no methods and the
RoomParticipantQueryRepositoryImpl class is an empty implementation, making it
unused. To fix this, either remove both the interface and its empty
implementation class if no custom queries are planned, or if custom queries are
needed later, define the required methods in the interface and implement them in
the class accordingly.
| @Entity | ||
| @Table(name = "room_participants") | ||
| @Getter | ||
| @SQLDelete(sql = "UPDATE room_participants SET status = 'INACTIVE' WHERE room_participant_id = ?") |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
소프트 삭제 구현이 올바르게 적용되었습니다
@SQLDelete 어노테이션을 통해 물리적 삭제 대신 status를 'INACTIVE'로 변경하는 소프트 삭제 방식이 구현되었습니다. 하지만 기존 조회 쿼리들이 활성 상태의 참가자만 조회하도록 수정되었는지 확인이 필요합니다.
다음 스크립트로 기존 쿼리들이 status 필터링을 올바르게 처리하는지 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# 설명: RoomParticipant 관련 쿼리들이 status 필터링을 올바르게 처리하는지 확인
# RoomParticipant 관련 repository 메서드들 검색
rg -A 10 "findBy.*RoomParticipant|existBy.*RoomParticipant" --type java
# Native query에서 status 조건 처리 확인
rg -A 5 -B 5 "SELECT.*room_participants|FROM.*room_participants" --type javaLength of output: 2928
조회 메서드에 상태 필터 추가 필요
소프트 삭제(@SQLDelete)는 올바르게 적용되었으나, 아래 조회 로직에 status = 'ACTIVE' 필터가 빠져 있어 비활성(INACTIVE) 참가자까지 조회될 수 있습니다.
– src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java
• findByUserIdAndRoomId
- 기존
java @Query(value = "SELECT * FROM room_participants WHERE user_id = :userId AND room_id = :roomId", nativeQuery = true) Optional<RoomParticipantJpaEntity> findByUserIdAndRoomId(...);
- 수정 제안
java @Query(value = "SELECT * FROM room_participants WHERE user_id = :userId AND room_id = :roomId AND status = 'ACTIVE'", nativeQuery = true)
• findAllByRoomJpaEntity_RoomId
- 파생 쿼리에 상태 조건이 누락되어 있습니다.
- 방법 1: 메서드 시그니처를 findAllByRoomJpaEntity_RoomIdAndStatus(Long roomId, Status status)로 변경
- 방법 2: 엔티티에 @Where(clause = "status = 'ACTIVE'")를 추가하여 전역 필터 적용
existByUserIdAndRoomId는 이미 ... AND rp.status = 'ACTIVE' 조건이 포함되어 있어 추가 변경이 필요 없습니다.
위 두 조회 로직에 활성 상태 필터를 반영해 주세요.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java
around the methods findByUserIdAndRoomId and findAllByRoomJpaEntity_RoomId, the
queries lack a filter for status = 'ACTIVE', causing inactive participants to be
included in results. To fix this, update the findByUserIdAndRoomId query to add
"AND status = 'ACTIVE'" in the WHERE clause, and for
findAllByRoomJpaEntity_RoomId, either modify the method signature to include a
status parameter and filter by it or add @Where(clause = "status = 'ACTIVE'")
annotation on the entity to globally filter by active status. No changes are
needed for existByUserIdAndRoomId as it already includes the status filter.
| boolean exists = roomParticipantJpaRepository | ||
| .existByUserIdAndRoomId(participant.getUserId(), room.getRoomId()); | ||
| assertThat(exists).isFalse(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
컴파일 에러 수정 필요
existByUserIdAndRoomId 메서드의 반환 타입이 boolean이 아닌 것으로 보입니다. 파이프라인 에러에 따르면 Long을 boolean으로 변환할 수 없다고 나오고 있습니다.
메서드 시그니처를 확인하고 다음 중 하나로 수정해야 합니다:
- boolean exists = roomParticipantJpaRepository
- .existByUserIdAndRoomId(participant.getUserId(), room.getRoomId());
+ boolean exists = roomParticipantJpaRepository
+ .existsByUserIdAndRoomId(participant.getUserId(), room.getRoomId());또는 다른 적절한 메서드를 사용해야 합니다.
🏁 Script executed:
#!/bin/bash
# 저장소에서 RoomParticipantJpaRepository의 메서드 시그니처 확인
ast-grep --pattern $'interface RoomParticipantJpaRepository {
$$$
$_($_) $_
$$$
}'Length of output: 89
다음 스크립트를 실행해 RoomParticipantJpaRepository의 정의와 existByUserIdAndRoomId/existsByUserIdAndRoomId 메서드 시그니처를 확인해주세요.
🏁 Script executed:
#!/bin/bash
# RoomParticipantJpaRepository 인터페이스 정의 확인
fd --extension java RoomParticipantJpaRepository.java --exec sed -n '1,200p' {}
# 메서드 이름 검색
rg -n 'existByUserIdAndRoomId|existsByUserIdAndRoomId'Length of output: 3074
RoomJoinApiTest 컴파일 에러 수정 필요
RoomJoinApiTest에서 JPA 리포지토리 메서드 시그니처(Long existByUserIdAndRoomId(...))와 맞지 않게 boolean으로 직접 받고 있어 컴파일 에러가 발생합니다. 아래와 같이 테스트 코드를 변경해 주세요:
파일: src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java
수정 위치: 180–182
- boolean exists = roomParticipantJpaRepository
- .existByUserIdAndRoomId(participant.getUserId(), room.getRoomId());
- assertThat(exists).isFalse();
+ Optional<RoomParticipantJpaEntity> participantOpt = roomParticipantJpaRepository
+ .findByUserIdAndRoomId(participant.getUserId(), room.getRoomId());
+ assertThat(participantOpt).isEmpty();– 또는 원하시는 경우 숫자 0/1 비교 방식으로도 변경할 수 있습니다:
- boolean exists = roomParticipantJpaRepository
- .existByUserIdAndRoomId(participant.getUserId(), room.getRoomId());
- assertThat(exists).isFalse();
+ Long count = roomParticipantJpaRepository
+ .existByUserIdAndRoomId(participant.getUserId(), room.getRoomId());
+ assertThat(count).isZero();위 중 한 가지 방식으로 수정하시면 테스트가 정상 컴파일 및 실행됩니다.
📝 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.
| boolean exists = roomParticipantJpaRepository | |
| .existByUserIdAndRoomId(participant.getUserId(), room.getRoomId()); | |
| assertThat(exists).isFalse(); | |
| Long count = roomParticipantJpaRepository | |
| .existByUserIdAndRoomId(participant.getUserId(), room.getRoomId()); | |
| assertThat(count).isZero(); |
🧰 Tools
🪛 GitHub Actions: CI with Gradle
[error] 181-181: Compilation error: incompatible types: Long cannot be converted to boolean at line 181.
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java around
lines 180 to 182, the method existByUserIdAndRoomId returns a Long but the test
expects a boolean, causing a compile error. Fix this by changing the test to
receive a Long result and compare it to 0 (e.g.,
assertThat(result).isEqualTo(0L)) or convert the Long to boolean by checking if
it is greater than 0. Update the assertion accordingly to match the method's
return type.
| @Query( | ||
| value = "SELECT EXISTS (SELECT 1 FROM room_participants rp WHERE rp.user_id = :userId AND rp.room_id = :roomId AND rp.status = 'ACTIVE')", | ||
| nativeQuery = true | ||
| ) | ||
| Long existByUserIdAndRoomId(@Param("userId") Long userId, @Param("roomId") Long roomId); |
There was a problem hiding this comment.
메서드 반환 타입과 네이밍 수정 필요
- EXISTS 쿼리는 boolean을 반환해야 합니다
- 메서드명이
existBy...가 아닌existsBy...여야 합니다
-@Query(
- value = "SELECT EXISTS (SELECT 1 FROM room_participants rp WHERE rp.user_id = :userId AND rp.room_id = :roomId AND rp.status = 'ACTIVE')",
- nativeQuery = true
-)
-Long existByUserIdAndRoomId(@Param("userId") Long userId, @Param("roomId") Long roomId);
+boolean existsByUserIdAndRoomIdAndStatus(Long userId, Long roomId, String status);또는 @Query를 사용한다면:
-Long existByUserIdAndRoomId(@Param("userId") Long userId, @Param("roomId") Long roomId);
+boolean existsByUserIdAndRoomId(@Param("userId") Long userId, @Param("roomId") Long roomId);📝 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.
| @Query( | |
| value = "SELECT EXISTS (SELECT 1 FROM room_participants rp WHERE rp.user_id = :userId AND rp.room_id = :roomId AND rp.status = 'ACTIVE')", | |
| nativeQuery = true | |
| ) | |
| Long existByUserIdAndRoomId(@Param("userId") Long userId, @Param("roomId") Long roomId); | |
| @Query( | |
| value = "SELECT EXISTS (SELECT 1 FROM room_participants rp WHERE rp.user_id = :userId AND rp.room_id = :roomId AND rp.status = 'ACTIVE')", | |
| nativeQuery = true | |
| ) | |
| boolean existsByUserIdAndRoomId(@Param("userId") Long userId, @Param("roomId") Long roomId); |
| @Query( | |
| value = "SELECT EXISTS (SELECT 1 FROM room_participants rp WHERE rp.user_id = :userId AND rp.room_id = :roomId AND rp.status = 'ACTIVE')", | |
| nativeQuery = true | |
| ) | |
| Long existByUserIdAndRoomId(@Param("userId") Long userId, @Param("roomId") Long roomId); | |
| boolean existsByUserIdAndRoomIdAndStatus(Long userId, Long roomId, String status); |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java
lines 18 to 22, the method uses a native EXISTS query but returns Long and is
named existByUserIdAndRoomId. Change the return type to boolean and rename the
method to existsByUserIdAndRoomId to correctly reflect the boolean result of the
EXISTS query.
| Room room; | ||
| try { | ||
| room = roomCommandPort.findById(roomJoinCommand.roomId()); | ||
| } catch (EntityNotFoundException e) { | ||
| throw new InvalidStateException(ErrorCode.USER_CANNOT_JOIN_OR_CANCEL); | ||
| } | ||
|
|
||
| boolean isParticipate = roomParticipantQueryPort.existByUserIdAndRoomId(roomJoinCommand.userId(), roomJoinCommand.roomId()); | ||
| room.validateRoomExpired(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
검증 순서 조정 및 구체적인 에러 처리 필요
방의 만료 상태를 먼저 확인한 후 참여 상태를 확인하는 것이 더 논리적입니다:
Room room;
try {
room = roomCommandPort.findById(roomJoinCommand.roomId());
+ room.validateRoomExpired(); // 만료 검증을 먼저 수행
} catch (EntityNotFoundException e) {
- throw new InvalidStateException(ErrorCode.USER_CANNOT_JOIN_OR_CANCEL);
+ throw new EntityNotFoundException(ErrorCode.ROOM_NOT_FOUND);
}
boolean isParticipate = roomParticipantQueryPort.existByUserIdAndRoomId(roomJoinCommand.userId(), roomJoinCommand.roomId());
-room.validateRoomExpired();📝 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.
| Room room; | |
| try { | |
| room = roomCommandPort.findById(roomJoinCommand.roomId()); | |
| } catch (EntityNotFoundException e) { | |
| throw new InvalidStateException(ErrorCode.USER_CANNOT_JOIN_OR_CANCEL); | |
| } | |
| boolean isParticipate = roomParticipantQueryPort.existByUserIdAndRoomId(roomJoinCommand.userId(), roomJoinCommand.roomId()); | |
| room.validateRoomExpired(); | |
| Room room; | |
| try { | |
| room = roomCommandPort.findById(roomJoinCommand.roomId()); | |
| room.validateRoomExpired(); // 만료 검증을 먼저 수행 | |
| } catch (EntityNotFoundException e) { | |
| throw new EntityNotFoundException(ErrorCode.ROOM_NOT_FOUND); | |
| } | |
| boolean isParticipate = roomParticipantQueryPort.existByUserIdAndRoomId( | |
| roomJoinCommand.userId(), | |
| roomJoinCommand.roomId() | |
| ); |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/room/application/service/RoomJoinService.java
around lines 34 to 42, the current code checks if the user participates in the
room before validating if the room is expired. Change the order to first
validate the room's expiration status by calling room.validateRoomExpired()
immediately after retrieving the room, then check the user's participation
status. This ensures logical flow by verifying the room's validity before
checking user participation.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java (1)
23-23: 메서드명 수정 필요메서드명이
existByUserIdAndRoomId가 아닌existsByUserIdAndRoomId여야 합니다. Spring Data JPA 명명 규칙을 따라야 합니다.-boolean existByUserIdAndRoomId(@Param("userId") Long userId, @Param("roomId") Long roomId); +boolean existsByUserIdAndRoomId(@Param("userId") Long userId, @Param("roomId") Long roomId);
🧹 Nitpick comments (1)
src/test/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntityTest.java (1)
46-47: 테스트 데이터 개선이 잘 적용되었습니다.명시적인 ISBN 값을 사용하여 테스트의 예측 가능성과 디버깅 용이성이 향상되었습니다.
TestEntityFactory.createBookWithISBN()메서드를 적절히 활용하고 있습니다.참고로 현재 사용된 ISBN "1234567890"은 10자리 형식입니다. 만약 일관성을 위해 13자리 ISBN-13 형식을 사용하고 싶다면 "1234567890123"과 같이 변경하는 것을 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantCommandPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java(1 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java(4 hunks)src/test/java/konkuk/thip/record/adapter/in/web/RecordCreateControllerTest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomGetHomeJoinedRoomsApiTest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomGetMemberListApiTest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomPlayingDetailViewApiTest.java(7 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomRecruitingDetailViewApiTest.java(5 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomSearchApiTest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntityTest.java(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/test/java/konkuk/thip/room/adapter/in/web/RoomPlayingDetailViewApiTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantQueryPersistenceAdapter.java
- src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java
🧰 Additional context used
🧠 Learnings (3)
src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java (1)
Learnt from: hd0rable
PR: THIP-TextHip/THIP-Server#57
File: src/test/java/konkuk/thip/room/domain/RoomTest.java:0-0
Timestamp: 2025-07-08T16:30:33.771Z
Learning: Room 도메인에서 startDate는 현재 날짜 이후여야 하는 도메인 규칙이 있어서, 테스트에서 만료된 상태를 시뮬레이션하려면 reflection을 사용해야 한다.
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java (2)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantCommandPersistenceAdapter.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
🧬 Code Graph Analysis (2)
src/test/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntityTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(21-172)
src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(21-172)
⏰ 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 (14)
src/test/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntityTest.java (1)
65-65: 검증 로직이 올바르게 개선되었습니다.명시적으로 설정한 ISBN 변수를 사용하여 일관성 있는 테스트 검증이 이루어지고 있습니다. 이는 테스트 설정과 검증 단계 간의 일관성을 보장하는 좋은 방법입니다.
src/test/java/konkuk/thip/room/adapter/in/web/RoomGetMemberListApiTest.java (1)
128-128: 배치 삭제로 변경하여 테스트 성능 개선됨
deleteAll()에서deleteAllInBatch()로 변경하여 테스트 정리 단계에서 개별 DELETE 문 대신 단일 배치 DELETE 문을 실행하도록 최적화했습니다.src/test/java/konkuk/thip/record/adapter/in/web/RecordCreateControllerTest.java (1)
77-77: 배치 삭제로 변경하여 테스트 성능 개선됨다른 테스트 파일들과 동일하게
deleteAllInBatch()로 변경하여 테스트 정리 단계에서의 성능을 개선했습니다.src/test/java/konkuk/thip/room/adapter/in/web/RoomGetHomeJoinedRoomsApiTest.java (1)
96-96: 배치 삭제로 변경하여 테스트 성능 개선됨테스트 정리 단계에서
deleteAllInBatch()를 사용하여 성능을 개선했습니다.src/test/java/konkuk/thip/room/adapter/in/web/RoomSearchApiTest.java (1)
65-65: 배치 삭제로 변경하여 테스트 성능 개선됨테스트 정리 단계에서 배치 삭제를 사용하여 성능을 개선했습니다.
src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java (3)
3-26: Import 문 정리 및 추가필요한 import 문들이 추가되고 정리되었습니다. 코드 가독성과 구조화에 도움이 됩니다.
125-125: 배치 삭제로 변경하여 테스트 성능 개선됨테스트 정리 단계에서
deleteAllInBatch()를 사용하여 성능을 개선했습니다.
156-156: 테스트 중간 정리에서도 배치 삭제 사용테스트 메서드 내에서도 배치 삭제를 사용하여 성능을 개선했습니다. 테스트 시나리오 준비 과정에서 효율적인 데이터 정리가 이루어집니다.
src/test/java/konkuk/thip/room/adapter/in/web/RoomRecruitingDetailViewApiTest.java (2)
66-66: 배치 삭제로 성능 개선
deleteAllInBatch()사용으로 테스트 정리 시 성능이 향상됩니다.
166-166: 새로운 네이티브 쿼리 메서드 사용새로운
findAllByRoomId()메서드로 변경하여 네이티브 쿼리 기반 조회를 정확히 활용하고 있습니다.Also applies to: 214-214, 267-267, 319-319
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/roomparticipant/RoomParticipantJpaRepository.java (1)
13-17: 네이티브 쿼리와 ACTIVE 상태 필터링Soft delete 구현을 위한 ACTIVE 상태 필터링이 올바르게 적용되어 있습니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomParticipantCommandPersistenceAdapter.java (3)
34-34: 새로운 리포지토리 메서드 사용새로운 네이티브 쿼리 기반 메서드로 올바르게 변경되었습니다.
Also applies to: 43-43
48-60: 엔티티 검증과 저장 로직사용자와 방 엔티티의 존재 여부를 먼저 검증한 후 참가자를 저장하는 올바른 구현입니다. 적절한 예외 처리가 포함되어 있습니다.
62-69: 참가자 삭제 로직참가자의 존재를 확인한 후 삭제하는 안전한 구현입니다. 예외 처리가 적절히 적용되어 있습니다.
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
고생하셨습니다! 리뷰 확인 부탁드립니다!
|
|
||
| import konkuk.thip.room.application.port.in.dto.RoomJoinCommand; | ||
|
|
||
| public interface RoomJoinUsecase { |
| //Room의 memberCount 업데이트 | ||
| room.increaseMemberCount(); |
There was a problem hiding this comment.
p3 : 저희가 개발시에 빼먹을 수도 있는 부분이라고 생각 + RoomParticipant와는 다른 애그리거트인 Room의 상태변경이 이루어진다고 생각하는데, 추후에 어떻게 최적화를 할 수 있을지(ex. event driven) 고민해보면 좋을거 같습니다!
There was a problem hiding this comment.
다른 애그리거트이기 때문에 이벤트 처리 기반으로 상태 변경을 하자는 말씀이신가요..?
There was a problem hiding this comment.
조금 덧붙이자면, 다른 애그리거트이기 때문에 분리해보자 라기보다는 해당 로직처럼 여러 애그리거트가 업데이트 되어야하는 플로우는 저희가 (특히 제가) 로직을 빼먹거나 놓칠 가능성이 있다고 생각하는데, 이걸 개선할 방법이 없을까? 라는 생각을 해보자 라는 말이었습니다
| .userPercentage(0.0) | ||
| .userId(userId) | ||
| .roomId(roomId) | ||
| .roomParticipantRole(roomParticipantRole) |
There was a problem hiding this comment.
p3 : roomParticipantRole 의 타입을 String 으로 정의하신 이유가 있나요??
RoomParticipant 도메인은 String 타입의 role 을 가지고, RoomParticipant jpa 엔티티는 enum 타입의 role을 가지고, enum의 유효성 검사는 enum 을 생성하는 시점에서 이루어지는데
이러면 잘못된 role 값을 가진 도메인은 도메인 생성 시점이 아니라, mapper에 의해서 jpa로 변환되는 과정에서 exception이 발생하는 상황이 발생할 것 같습니다
(그런데 role을 request로 받는게 아니라, 우리가 상황에 맞춰서 enum.getValue() 로 넣어주는거니 좀 애매한거 같기도,,)
저는 jpa, 도메인 사이의 의존성이 생기더라도 RoomParticipant 도메인의 role의 타입을 RoomParticipantRole 로 정의하는게 어떨까 싶습니다!
이러면 member, host 별로 다른 도메인 로직을 구현할 때 String의 equals 비교를 통한 분기처리가 아니라, enum 내부로 역할별로 도메인 로직을 캡슐화할 수 있을 것 같습니다
There was a problem hiding this comment.
이 부분은 이전에 성준님이 작성하신 PR에서 도메인 엔티티의 외부 의존성을 최소화하기 위해 enum 타입을 도메인에서는 사용하지 않고 문자열(String)로 보유하는 방식으로 구현하셨던 것으로 기억합니다. 저 역시 같은 맥락에서 RoomParticipant에서도 RoomParticipantRole enum에 직접 의존하지 않고, 문자열로 역할 값을 처리하도록 구현했습니다.
다만, 저도 구현 과정에서 “enum과 같은 상수 타입도 도메인에서는 의존을 피하는 것이 맞는가?” 에 대해 고민이 들었던 상황이었습니다. 그렇다면 도메인에서도 enum 타입을 사용하는 방향으로 통일할까요?
There was a problem hiding this comment.
안그래도 리뷰달면서 제가 예전에 도메인 계층의 코드에 일부러 String 타입을 정의하는식으로 코드를 짰었는데 그게 어디있지했는데 역시 예리하시군요
처음에 도메인 엔티티와 jpa 를 분리하는 시점에서는 일단 전부 분리를 하자는 생각에 나눴었는데, 현준님 말처럼 어차피 저희가 지정한 값을 편하게 사용하기 위한 상수 타입도 분리하는게 맞는건지는 생각해볼 필요가 있는거 같네요
오히려 역할별 로직을 처리하기위해 도메인이 상수 타입을 의존하고, jpa는 단순 DB 저장이니 String (어차피 jpa 어노테이션을 활용해 String으로 변환해서 저장하니) 타입이 맞지않나 라는 생각도 드네요
우선순위를 고려해서 정해보면 좋을거 같습니다!
| throw new InvalidStateException(ErrorCode.USER_CANNOT_JOIN_OR_CANCEL); | ||
| } | ||
|
|
||
| boolean isParticipate = roomParticipantQueryPort.existByUserIdAndRoomId(roomJoinCommand.userId(), roomJoinCommand.roomId()); |
There was a problem hiding this comment.
p3 : 어차피 RoomParticipant 도메인이 필요하니 boolean 으로 roomParticipant의 존재여부를 조회하는 것보다는, 처음부터 RoomParticipant 도메인 자체를 조회하는건 어떤가요?
There was a problem hiding this comment.
초기에는 요청 타입에 따라 분기한 뒤 필요한 경우에만 도메인을 조회하는 방식이 도메인의 의도를 명확히 드러낸다고 판단하여 그렇게 구현하였는데, 리뷰 주신 내용을 바탕으로 다시 검토해보니, Optional로 한 번만 조회하고, 존재 여부에 따라 분기 처리하는 방식도 흐름이 명확하고 불필요한 중복 쿼리를 줄일 수 있다는 점에서 장점이 있을 것 같네요.
말씀 주신대로 findBy를 통해 Optional을 반환하도록 리팩토링해보겠습니다!
There was a problem hiding this comment.
현재 Port 에서 제공하는 findById 메서드가 Optional 이 아니라면(저희 도메인을 조회하는 메서드는 전부 도메인 객체가 반환타입 + NOT_FOUND exception throw 플로우였으니), try-catch를 활용하는 방법도 있을 것 같습니다!
| // 방장이 참여 취소를 요청한 경우 | ||
| RoomParticipant roomParticipant = roomParticipantCommandPort.findByUserIdAndRoomId(roomJoinCommand.userId(), roomJoinCommand.roomId()); | ||
| roomParticipant.validateHostCancelRoom(); | ||
|
|
||
| roomParticipantCommandPort.deleteByUserIdAndRoomId(roomJoinCommand.userId(), roomJoinCommand.roomId()); |
There was a problem hiding this comment.
p3 : RoomParticipant 도메인이 '방장이 참여 취소를 요청했는지를 검사한다' 라는 메서드를 제공하기 보다는, '해당 참여자의 참여 취소를 한다' 라는 메서드를 제공하고, 이 메서드 내부에서 '방장이면 exception throw' 라는 로직을 수행하는 플로우는 어떤가요?
이러면 서비스는
RoomParticipant 한테 '참여 취소' 요청을 보내고, update 된 도메인을 DB에 반영한다
라는 단순한 흐름을 유지할 수 있을 것 같습니다
There was a problem hiding this comment.
넵 그게 좀더 명확할 것 같네요! 수정하겠습니다
hd0rable
left a comment
hd0rable
left a comment
There was a problem hiding this comment.
수고하셨습니다! 간단한 리뷰 남겼는데 확인 부탁드립니당 🤩
| import konkuk.thip.room.application.port.in.dto.RoomJoinCommand; | ||
|
|
||
| public record RoomJoinRequest( | ||
| @NotBlank(message = "방 참여 유형 파라미터는 필수입니다..") |
| /** | ||
| * 방 생성 요청 | ||
| */ |
| MEMBER("팀원"); | ||
|
|
||
| private String type; | ||
| private final String type; |
| } | ||
|
|
||
| boolean isParticipate = roomParticipantQueryPort.existByUserIdAndRoomId(roomJoinCommand.userId(), roomJoinCommand.roomId()); | ||
| room.validateRoomExpired(); |
There was a problem hiding this comment.
p2: 코래 말처럼 검증순서를 방 만료 확인 -> RoomParticipant 도메인 존재 여부확인 순으로 하는 것은 어떨까요?
유즈케이스 관점에서 보면 “내가 이 방에 참여 가능한가?”를 판단하기 전에, 그 방 자체가 유효하고 참여 가능한 상태인가?를 먼저 확인하는것이 비지니스 로직에 맞다고 생각합니다.
또한, RoomParticipant 여부를 먼저 확인하면, 이미 참여 여부를 확인하는 DB 쿼리가 실행된 후에 방이 이미 만료된 상태임이 확인될 경우, 해당 RoomParticipant 쿼리는 필요 없는 호출이 될 것 같아요 DB 리소스 측면에서도 위와같은 방향으로 코드를 수정하는 것이 좋아 보입니다!
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
방 참여하기 / 취소하기 api 흐름은 다음과 같습니다.
type : "join"/"cancel"로 요청 -> 서비스 클래스에서 해당하는 방 조회 -> 참여하기 / 취소하기 요청 분기 처리
-> 참여하기 요청인 경우 RoomParticipant 도메인을 생성하여 insert 쿼리 후 Room의 memberCount 증가
-> 취소하기 요청인 경우 RoomParticipant을 조회하여 delete 쿼리 후 Room의 memberCount 감소
Error Response 정리
📸 스크린샷
💬 리뷰 요구사항
추가적으로, JPA 명명규칙에서 '_' 사용 관련해서 노션에 정리해두었으니 한번씩 읽어주세욥!
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
버그 수정
테스트
리팩터
기타