Conversation
…ssAndUpdateRoomProgress 작성 (#265)
Walkthrough방 나가기 기능을 추가하고 관련 예외 코드 및 Swagger 응답 설명을 확장했으며, 컨트롤러 엔드포인트, 유스케이스·서비스, 도메인 검증, 진행도 갱신 로직과 통합/단위 테스트를 도입했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Controller as RoomCommandController
participant Service as RoomParticipantDeleteService
participant RmCmd as RoomCommandPort
participant RmPartCmd as RoomParticipantCommandPort
participant Prog as RoomProgressManager
Client->>Controller: DELETE /rooms/{roomId}/leave (userId)
Controller->>Service: leaveRoom(userId, roomId)
Service->>RmCmd: findById(roomId)
RmCmd-->>Service: Room
Service->>RmPartCmd: findByUserIdAndRoomId(userId, roomId)
RmPartCmd-->>Service: RoomParticipant
Service->>Service: roomParticipant.validateRoomLeavable()
alt host 인 경우
Service-->>Controller: throw InvalidStateException(ROOM_HOST_CANNOT_LEAVE)
Controller-->>Client: 400 응답 (에러 코드)
else 참가자인 경우
Service->>Prog: removeUserProgressAndUpdateRoomProgress(roomParticipantId, Room)
Prog->>Prog: recalc avg progress, decrement memberCount, save Room
Prog-->>Service: done
Service->>RmPartCmd: deleteByUserIdAndRoomId(userId, roomId)
RmPartCmd-->>Service: done
Service-->>Controller: BaseResponse<Void>(success)
Controller-->>Client: 200 응답
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Results416 tests 416 ✅ 35s ⏱️ Results for commit ddb1cc3. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (15)
src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java (1)
88-89: 테스트용 setter는 유지하되, 가독성 개선을 권장합니다@VisibleForTesting 사용은 적절합니다. 다만 한 줄 메서드는 가독성이 떨어져 코드 스타일상 다중 라인으로 정리하면 좋습니다.
적용 제안 diff:
- @VisibleForTesting - public void updateRoomPercentage(double roomPercentage) {this.roomPercentage = roomPercentage;} + @VisibleForTesting + public void updateRoomPercentage(double roomPercentage) { + this.roomPercentage = roomPercentage; + }src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
144-144: 상태코드 재검토 제안: HOST 퇴장 금지의 HTTP 의미론ROOM_HOST_CANNOT_LEAVE가 BAD_REQUEST(400)로 정의되어 있는데, 권한/허가 측면의 도메인 제약이라면 FORBIDDEN(403)이 더 자연스러울 수 있습니다. 클라이언트가 파라미터를 잘못 보낸 상황이 아니라, 유저 상태(호스트)로 인해 해당 액션이 금지된 경우이기 때문입니다. 기존 에러코드들과의 일관성, 클라이언트 처리 로직(리트라이/UX)을 고려하여 선택해 주세요. 큰 문제는 아니므로 선택 사항입니다.
src/main/java/konkuk/thip/room/application/port/in/RoomParticipantDeleteUseCase.java (1)
1-5: 리턴 타입을 Void에서 void로 단순화하는 것을 권장합니다사용자/방 ID를 받아 부수효과만 수행하는 유스케이스는 반환값이 없으므로 primitive void가 자연스럽습니다. Void는 제네릭 호환성을 위해 선택되곤 하지만 포트 계층에는 불필요한 오버헤드이자 의미적 혼동을 줄 수 있습니다. 컨트롤러의 BaseResponse 사용과도 충돌하지 않습니다(유스케이스는 void여도 무방).
제안 diff:
package konkuk.thip.room.application.port.in; public interface RoomParticipantDeleteUseCase { - Void leaveRoom(Long userId, Long roomId); + void leaveRoom(Long userId, Long roomId); }src/main/java/konkuk/thip/roompost/application/service/RecordDeleteService.java (2)
8-8: 사용되지 않는 의존성(RoomProgressManager) 주입 제거 또는 즉시 사용으로 전환 권장현재 RoomProgressManager를 주입만 하고 사용하지 않습니다. 불필요한 의존성 주입은 테스트/리팩터링 시 잡음을 유발합니다. 이번 PR 범위에 사용하지 않는다면 제거하는 편이 깔끔합니다.
아래와 같이 import 및 필드를 제거할 수 있습니다:
-import konkuk.thip.roompost.application.service.manager.RoomProgressManager; @@ - private final RoomProgressManager roomProgressManager;Also applies to: 24-24
46-46: TODO 주석 대신 추적 이슈로 전환하거나 구현 시점을 명시하세요삭제 후 진행도 업데이트는 중요한 후속 작업입니다. TODO만 남기면 누락될 수 있으니, 이슈 번호(예: THIP2025-XXX)를 달거나 본 PR에서 제외 이유/대상 PR을 주석으로 명시하는 것을 권장합니다. 필요하면 구현 스켈레톤까지 도와드릴 수 있습니다.
src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java (1)
22-43: 중복 로직 DRY: 평균 진행률 계산 공통화 제안updateUserAndRoomProgress와 본 메서드 모두 방 평균 계산 로직이 유사합니다. 사적 유틸 메서드로 평균 계산을 분리하면 유지보수성이 좋아집니다.
src/test/java/konkuk/thip/room/domain/RoomParticipantTest.java (1)
3-3: 단언 스타일 일관화(선택)동일 클래스에서 AssertJ와 JUnit 단언을 혼용하고 있습니다. 팀 컨벤션에 따라 한 가지로 통일하면 가독성이 좋아집니다. 예: assertThatThrownBy/doesNotThrow를 사용하거나, JUnit만 사용.
Also applies to: 7-7, 9-9, 88-109
src/main/java/konkuk/thip/room/adapter/in/web/RoomCommandController.java (3)
111-114: PathVariable 이름을 명시해 바인딩 안정성과 일관성 확보컴파일 옵션에 -parameters 설정이 없으면 파라미터 이름 기반 매핑이 동작하지 않을 수 있습니다. 같은 클래스의 다른 메서드(likeRoomPost)가 명시적으로 이름을 부여하는 것과도 일관성을 맞추면 좋습니다.
아래처럼 수정 제안합니다.
- @Parameter(description = "나갈 방의 ID", example = "1") @PathVariable final Long roomId) { + @Parameter(description = "나갈 방의 ID", example = "1") @PathVariable("roomId") final Long roomId) {
111-115: 메서드 명을 목적에 맞게 단순화 제안핸들러 이름을 라우트/유스케이스와 동일하게 맞추면 가독성이 올라갑니다. deleteRoomParticipant → leaveRoom.
- public BaseResponse<Void> deleteRoomParticipant( + public BaseResponse<Void> leaveRoom(
111-115: DELETE 응답은 204 No Content 고려 (선택)현재 BaseResponse 패턴을 유지해야 한다면 OK입니다. 다만 REST 관점에서는 본문 없는 204가 자연스럽습니다. 추후 BaseResponse.ok()의 무데이터 오버로드(예: ok())를 제공하거나 ResponseEntity.noContent()를 사용하는 형태를 고려해볼 수 있습니다.
src/main/java/konkuk/thip/room/application/service/RoomParticipantDeleteService.java (2)
30-33: 참여자 검증 로직, RoomParticipantPolicy로 캡슐화 고려프로젝트 선호도(레퍼런스: RoomParticipantPolicy)에 맞춰 "사용자가 방에 속하는지 검증"을 정책/도메인 서비스로 캡슐화하면 재사용성과 일관성이 높아집니다. 현재 구현도 문제는 없으나, 동일 검증이 여러 유스케이스에서 반복될 경우 중복을 줄일 수 있습니다.
24-24: Void 반환 대신 void 사용 제안불필요한 null 반환을 제거해 의도를 명확히 할 수 있습니다. 포트 인터페이스와 컨트롤러의 제네릭도 함께 조정되어야 합니다.
- public Void leaveRoom(Long userId, Long roomId) { + public void leaveRoom(Long userId, Long roomId) { ... - return null; + return;Also applies to: 39-40
src/test/java/konkuk/thip/room/adapter/in/web/RoomParticipantDeleteApiTest.java (3)
32-32: AssertJ 정적 임포트 표준화 제안AssertionsForClassTypes보다 일반 Assertions가 더 보편적이며 제네릭 추론도 자연스럽습니다.
-import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.assertj.core.api.Assertions.assertThat;
135-138: MockMvc 호출 이후 flush/clear로 가시성/일관성 개선 (선택)동일 트랜잭션 내에서도 JPA 1차 캐시 영향으로 상태 가시성이 헷갈릴 수 있습니다. 검증 전 flush/clear를 호출하면 검사 일관성이 좋아집니다.
.andExpect(status().isOk()); + em.flush(); + em.clear(); + // 방에서 참여자 정보가 사라졌는지 확인 boolean exists = roomParticipantJpaRepository.existsByUserIdAndRoomId(participant.getUserId(), room.getRoomId());
148-171: 엣지 케이스 테스트 추가 제안
- 존재하지 않는 roomId에 대한 요청 → 404 ROOM_NOT_FOUND 기대
- 동일 사용자가 연속으로 나가기 요청 → 2번째 요청은 404(비참여자) 기대
필요 시 테스트 케이스 초안을 제공할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java(1 hunks)src/main/java/konkuk/thip/room/adapter/in/web/RoomCommandController.java(3 hunks)src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java(1 hunks)src/main/java/konkuk/thip/room/application/port/in/RoomParticipantDeleteUseCase.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomParticipantCommandPort.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomParticipantDeleteService.java(1 hunks)src/main/java/konkuk/thip/room/domain/RoomParticipant.java(2 hunks)src/main/java/konkuk/thip/roompost/application/service/RecordDeleteService.java(3 hunks)src/main/java/konkuk/thip/roompost/application/service/VoteDeleteService.java(1 hunks)src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomParticipantDeleteApiTest.java(1 hunks)src/test/java/konkuk/thip/room/domain/RoomParticipantTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-26T06:09:00.850Z
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 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
Applied to files:
src/main/java/konkuk/thip/room/application/service/RoomParticipantDeleteService.java
📚 Learning: 2025-08-14T12:29:00.150Z
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#183
File: src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java:33-45
Timestamp: 2025-08-14T12:29:00.150Z
Learning: 방 생성 시 항상 HOST 역할의 RoomParticipant가 생성되며, RoomProgressHelper.updateUserAndRoomProgress() 메서드는 사용자가 해당 방의 참여자임이 사전 검증된 상황에서만 호출되므로 참여자 목록이 비어있을 수 없다.
Applied to files:
src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java
🧬 Code Graph Analysis (4)
src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java (1)
src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.java (1)
VisibleForTesting(50-53)
src/main/java/konkuk/thip/room/domain/RoomParticipant.java (2)
src/main/java/konkuk/thip/room/domain/Room.java (1)
Getter(16-185)src/main/java/konkuk/thip/room/domain/RoomParticipants.java (1)
Getter(12-55)
src/test/java/konkuk/thip/room/adapter/in/web/RoomParticipantDeleteApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(29-413)
src/test/java/konkuk/thip/room/domain/RoomParticipantTest.java (1)
src/test/java/konkuk/thip/room/domain/RoomParticipantsTest.java (1)
DisplayName(12-116)
⏰ 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 (8)
src/main/java/konkuk/thip/room/application/port/out/RoomParticipantCommandPort.java (1)
9-10: static import 전환 및 예외 매핑 깔끔합니다ErrorCode에 대한 static import로 가독성이 좋아졌고, 기본 메서드의 예외 매핑도 명확합니다. 추가 액션 없습니다.
Also applies to: 17-18
src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java (1)
120-124: ROOM_LEAVE 에러 스펙 정의 적절합니다방 나가기 플로우에서 발생 가능한 도메인 에러(ROOM_NOT_FOUND, ROOM_PARTICIPANT_NOT_FOUND, ROOM_HOST_CANNOT_LEAVE)를 잘 반영했습니다. 공통 API 에러는 생성자에서 주입되므로 누락 위험도 없습니다.
src/main/java/konkuk/thip/room/domain/RoomParticipant.java (1)
74-78: 호스트 퇴장 방지 도메인 규칙 추가 적합isHost() 체크로 호스트 퇴장을 명확히 차단합니다. 예외 타입/코드도 일관적입니다.
src/test/java/konkuk/thip/room/domain/RoomParticipantTest.java (1)
88-109: 호스트/멤버 퇴장 도메인 테스트 추가 적절호스트 퇴장 불가 및 멤버 퇴장 가능 케이스를 명확히 커버합니다. 에러코드 단언도 좋아요.
src/main/java/konkuk/thip/room/adapter/in/web/RoomCommandController.java (1)
105-110: 엔드포인트 및 스웨거 메타데이터 구성 LGTM삭제(leave) API 라우팅과 예외 스웨거 매핑이 유스케이스와 일관되게 연결되어 있습니다. 컨트롤러 책임도 적절히 얇습니다.
src/main/java/konkuk/thip/room/application/service/RoomParticipantDeleteService.java (2)
22-40: 도메인 흐름과 트랜잭션 경계가 명확함
- 방 존재 확인 → 참여자 조회 → 권한 검증 → 진행도/멤버수 갱신 → 삭제 순서가 안정적입니다.
- @transactional 적용도 적절합니다.
35-38: 확인 완료: removeUserProgressAndUpdateRoomProgress에서 memberCount 감소 처리 포함조사 결과, RoomProgressManager.removeUserProgressAndUpdateRoomProgress 내부에 명시적으로 멤버 수를 감소시키는 로직이 있습니다.
– src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java:58room.decreaseMemberCount();따라서 RoomParticipantDeleteService에서 추가로 멤버 수를 감소시키는 호출은 불필요합니다.
src/test/java/konkuk/thip/room/adapter/in/web/RoomParticipantDeleteApiTest.java (1)
37-42: 핵심 성공/실패 시나리오 커버리지 충실성공, 비참여자(404), 호스트 불가(400)까지 필수 경로를 잘 커버하고 있습니다. 데이터 준비 유틸도 명확합니다.
src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java
Outdated
Show resolved
Hide resolved
| //TODO// 4. 유저 방 진행도 업데이트 | ||
| return command.roomId(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
투표 삭제 시 진행도 누락: TODO를 실제 로직으로 연결해 주세요
투표 삭제가 사용자/방 진행도 산정에 영향이 있다면, 현재 상태에서는 진행도가 갱신되지 않아 불일치가 발생할 수 있습니다. 본 PR의 목표(방 나가기)와 직접적 연관이 적더라도, 최소한 TODO를 추적 가능한 이슈로 전환하거나 실제 RoomProgressManager 연동을 추가하는 편이 안전합니다.
원하시면 RoomProgressManager 연동 패턴(주입, 트랜잭션 전후 위치, 예외 전파 기준)에 맞춰 구체 구현 스케치를 드리겠습니다.
다음 스크립트로 현재 코드베이스에 진행도 관련 매니저/메서드가 어떤 것이 있는지 빠르게 확인할 수 있습니다:
🏁 Script executed:
#!/bin/bash
# 진행도 매니저 및 관련 메서드 탐색
rg -n -C3 'class\s+RoomProgressManager\b|RoomProgressManager|progress' --type=java
# 삭제/감소류 메서드 단서 탐색
rg -n -C2 -e 'remove.*Progress|update.*Progress|decrease.*Progress' --type=javaLength of output: 43585
🏁 Script executed:
#!/bin/bash
# Locate VoteDeleteService and display context around TODO
rg -n -C3 'class\s\+VoteDeleteService' --type=java
# Show lines 1–80 of VoteDeleteService for full context
sed -n '1,80p' src/main/java/konkuk/thip/roompost/application/service/VoteDeleteService.javaLength of output: 1872
투표 삭제 후 진행도 업데이트 로직 누락 – RoomProgressManager 연결 필요
VoteDeleteService 에서 TODO로 남아 있는 “유저 방 진행도 업데이트”를 실제 로직으로 연결해야 합니다. 현재 삭제한 투표에 따른 사용자·방의 진행도 불일치가 발생할 수 있습니다.
- 조치 사항
VoteDeleteService에RoomProgressManager주입- 댓글/좋아요·투표 삭제 직후에
혹은 비즈니스에 맞게
roomProgressManager.removeUserProgressAndUpdateRoomProgress( /* 방참여자 ID(또는 userId→roomParticipant 조회) */, command.roomId() );
updateUserAndRoomProgress(userId, roomId, page)호출 추가 - 다른 서비스(
VoteCreateService,RecordDeleteService,RoomParticipantDeleteService)의 RoomProgressManager 연동 패턴 참고
위 변경을 통해 “TODO”를 추적 가능한 실제 이슈로 전환하거나 즉시 반영하여 방 진행도 일관성을 보장해 주세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/roompost/application/service/VoteDeleteService.java
around lines 44-45, the TODO for "유저 방 진행도 업데이트" must be implemented: inject
RoomProgressManager into this service (constructor field), and immediately after
removing the vote call the appropriate RoomProgressManager method (e.g.
removeUserProgressAndUpdateRoomProgress(participantId, command.roomId()) or
updateUserAndRoomProgress(userId, command.roomId(), page)) — obtain the room
participant id via existing userId→roomParticipant lookup if needed; follow the
same injection and invocation pattern used in
VoteCreateService/RecordDeleteService/RoomParticipantDeleteService so that
room/user progress stays consistent after vote deletion.
seongjunnoh
left a comment
There was a problem hiding this comment.
고생하셨습니다! 사소한 리뷰하나 남겨보았는데 의견 주시면 감사하겠습니다!
| public void validateRoomLeavable() { | ||
| if (isHost()) { | ||
| throw new InvalidStateException(ROOM_HOST_CANNOT_LEAVE); | ||
| } | ||
| } |
| public void removeUserProgressAndUpdateRoomProgress(Long removeRoomParticipantId, Long roomId) { | ||
|
|
||
| Room room = roomCommandPort.getByIdOrThrow(roomId); |
There was a problem hiding this comment.
p3 : 음 이미 application 에서 조회한 room domain entity 를 메서드 인자로 전달받으면, 현재 메서드에서 DB로부터 room domain entity 를 다시 조회하는 로직을 뺄 수 있을 것 같습니다!
RoomProgressManager는 room, roomParticipant 를 조회하는 것이 아니라,
전달받은 room, roomParticipant 도메인의 상태변경 & 이 변경사항의 영속화를 담당하는 책임만을 가지도록 강제하는 것이 더 좋아보여서 제안드립니다!
그런데 위에 updateUserAndRoomProgress 메서드에서도 id 값을 전달받아서 내부에서 한번 조회하고 있네여 허허
RoomProgressManager 를 도입하면서 별 생각이 없었네요ㅎ
There was a problem hiding this comment.
ㅎ 저두 위에 코드 보고 참고해서 작성하긴한건데 그럼위에코드도같이 수정할게용
There was a problem hiding this comment.
ㅋㅋㅋㅋ저도 같은 생각이긴 했는데 리팩 할래말래~
| // 나간 유저를 제외한 방 평균 진행률 update | ||
| List<RoomParticipant> remainingParticipants = roomParticipantCommandPort.findAllByRoomId(roomId); | ||
| double total = remainingParticipants.stream() | ||
| .filter(p -> !p.getId().equals(removeRoomParticipantId)) // 나간 유저 제외 | ||
| .mapToDouble(RoomParticipant::getUserPercentage) | ||
| .sum(); | ||
| room.updateRoomPercentage(total / (remainingParticipants.size() - 1)); |
| // 방 멤버 수 감소 | ||
| room.decreaseMemberCount(); | ||
| // 영속화 | ||
| roomCommandPort.update(room); |
| private void updateRoomPercentage() { | ||
| hostParticipation.updateCurrentPage(50); | ||
| hostParticipation.updateUserPercentage(50.0); // 방장의 진행도 50 | ||
| roomParticipantJpaRepository.save(hostParticipation); | ||
|
|
||
| memberParticipation.updateCurrentPage(30); | ||
| memberParticipation.updateUserPercentage(30.0); // 참여자 진행도 30 | ||
| roomParticipantJpaRepository.save(memberParticipation); | ||
|
|
||
| room.updateRoomPercentage(40); // 방참여자들의 진행도 평균 40 | ||
| roomJpaRepository.save(room); | ||
| } |
| @SpringBootTest | ||
| @ActiveProfiles("test") | ||
| @Transactional | ||
| @AutoConfigureMockMvc(addFilters = false) | ||
| @DisplayName("[통합] 방 나가기(멤버) API 통합 테스트") |
| @Test | ||
| @DisplayName("validateRoomLeavable: 방의 HOST가 방을 나가려고 하면 InvalidStateException이 발생한다.") | ||
| void host_cannot_leave_room_test() { | ||
| // given: HOST 참여자 생성 | ||
| RoomParticipant host = RoomParticipant.hostWithoutId(1L,1L); | ||
|
|
||
| // when & then | ||
| InvalidStateException ex = assertThrows(InvalidStateException.class, | ||
| host::validateRoomLeavable); | ||
| assertEquals(ROOM_HOST_CANNOT_LEAVE, ex.getErrorCode()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("MEMBER는 방을 나갈 수 있다.") | ||
| void member_can_leave_room_test() { | ||
| // given: MEMBER 참여자 생성 | ||
| RoomParticipant member = RoomParticipant.memberWithoutId(1L,1L); | ||
|
|
||
| // when & then | ||
| assertDoesNotThrow(member::validateRoomLeavable); | ||
| } | ||
|
|
| public void removeUserProgressAndUpdateRoomProgress(Long removeRoomParticipantId, Long roomId) { | ||
|
|
||
| Room room = roomCommandPort.getByIdOrThrow(roomId); | ||
|
|
||
| // 나간 유저를 제외한 방 평균 진행률 update | ||
| List<RoomParticipant> remainingParticipants = roomParticipantCommandPort.findAllByRoomId(roomId); | ||
| double total = remainingParticipants.stream() | ||
| .filter(p -> !p.getId().equals(removeRoomParticipantId)) // 나간 유저 제외 | ||
| .mapToDouble(RoomParticipant::getUserPercentage) | ||
| .sum(); | ||
| room.updateRoomPercentage(total / (remainingParticipants.size() - 1)); | ||
|
|
||
| // 방 멤버 수 감소 | ||
| room.decreaseMemberCount(); | ||
| // 영속화 | ||
| roomCommandPort.update(room); | ||
| } |
|
|
||
| @Override | ||
| @Transactional | ||
| public Void leaveRoom(Long userId, Long roomId) { | ||
|
|
||
| // 1. 방 조회 및 검증 | ||
| Room room = roomCommandPort.getByIdOrThrow(roomId); | ||
|
|
||
| // 2. 사용자가 방 참여자인지 확인 | ||
| RoomParticipant roomParticipant = roomParticipantCommandPort.getByUserIdAndRoomIdOrThrow(userId, room.getId()); | ||
| // 2-1. 방 나가기 권한 검증 | ||
| roomParticipant.validateRoomLeavable(); | ||
|
|
||
| // 3. 방 멤버수 감소 / 방 진행률 업데이트 | ||
| roomProgressManager.removeUserProgressAndUpdateRoomProgress(roomParticipant.getId(), room.getId()); | ||
|
|
||
| // 4. 방나가기 | ||
| roomParticipantCommandPort.deleteByUserIdAndRoomId(userId, room.getId()); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java (1)
39-53: 분모 계산 오류 및 0으로 나누기 가능성 — 필터링된 잔여 인원 수로 나누도록 수정 필요현재
remainingParticipants.size() - 1로 나누고 있어,
- 삭제 전/후 호출 순서에 따라 잘못된 분모를 사용할 수 있고
- 특히 이미 삭제된 상태에서 호출되면 0으로 나누기가 발생합니다.
필터링으로 “잔여 인원”을 먼저 확정하고, 해당 크기를 분모로 사용하세요. 잔여 0명일 때의 방어 코드도 포함하십시오.
- // 나간 유저를 제외한 방 평균 진행률 update - List<RoomParticipant> remainingParticipants = roomParticipantCommandPort.findAllByRoomId(room.getId()); - double total = remainingParticipants.stream() - .filter(p -> !p.getId().equals(removeRoomParticipantId)) // 나간 유저 제외 - .mapToDouble(RoomParticipant::getUserPercentage) - .sum(); - room.updateRoomPercentage(total / (remainingParticipants.size() - 1)); + // 나간 유저를 제외한 방 평균 진행률 update + List<RoomParticipant> remainingParticipants = roomParticipantCommandPort.findAllByRoomId(room.getId()) + .stream() + .filter(p -> !p.getId().equals(removeRoomParticipantId)) // 나간 유저 제외 + .toList(); + int remainingCount = remainingParticipants.size(); + double total = remainingParticipants.stream() + .mapToDouble(RoomParticipant::getUserPercentage) + .sum(); + room.updateRoomPercentage(remainingCount == 0 ? 0 : total / remainingCount);후속 권장 사항:
- 동일 플로우에서
room.decreaseMemberCount()가 중복 호출되지 않는지 호출 경로를 재확인하세요.- 테스트 케이스: (1) 삭제 전 호출, (2) 삭제 후 호출, (3) 잔여 0명(이론상 거의 불가) 시나리오 방어 확인.
🧹 Nitpick comments (1)
src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java (1)
26-33: 방어적 프로그래밍: 참여자 리스트가 비는 상황 대비 (저확률) 권장프로젝트 학습 기록에 따르면 방 생성 시 HOST가 존재하여 비어있지 않다는 전제이긴 하나, 극단적인 경합/데이터 이상 상황에서 0으로 나눌 가능성에 대한 최소 방어 코드를 두는 편이 안전합니다. 영향도 낮은 니트픽 수준 제안입니다.
아래처럼 0명일 때 0%로 설정하는 가드를 추가할 수 있습니다:
- List<RoomParticipant> all = roomParticipantCommandPort.findAllByRoomId(room.getId()); - double total = all.stream() + List<RoomParticipant> all = roomParticipantCommandPort.findAllByRoomId(room.getId()); + if (all.isEmpty()) { + room.updateRoomPercentage(0); + // 3. 영속화 + roomCommandPort.update(room); + roomParticipantCommandPort.update(roomParticipant); + return; + } + double total = all.stream() .filter(p -> !roomParticipant.getId().equals(p.getId())) // 현재 유저 제외 .mapToDouble(RoomParticipant::getUserPercentage) .sum(); total += roomParticipant.getUserPercentage(); room.updateRoomPercentage(total / all.size());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/main/java/konkuk/thip/room/application/service/RoomParticipantDeleteService.java(1 hunks)src/main/java/konkuk/thip/roompost/application/service/RecordCreateService.java(1 hunks)src/main/java/konkuk/thip/roompost/application/service/VoteCreateService.java(3 hunks)src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/thip/room/application/service/RoomParticipantDeleteService.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-14T12:29:00.150Z
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#183
File: src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java:33-45
Timestamp: 2025-08-14T12:29:00.150Z
Learning: 방 생성 시 항상 HOST 역할의 RoomParticipant가 생성되며, RoomProgressHelper.updateUserAndRoomProgress() 메서드는 사용자가 해당 방의 참여자임이 사전 검증된 상황에서만 호출되므로 참여자 목록이 비어있을 수 없다.
Applied to files:
src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java
📚 Learning: 2025-07-26T06:09:00.850Z
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 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
Applied to files:
src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java
⏰ 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 (3)
src/main/java/konkuk/thip/roompost/application/service/RecordCreateService.java (1)
64-64: 도메인 객체 기반 진행률 업데이트 호출로 일관성 개선 — OK
RoomParticipant,Room,Book도메인을 사전 조회하여RoomProgressManager.updateUserAndRoomProgress(...)에 전달하는 방식으로 리팩터링된 점 좋습니다. 호출부 책임이 명확해지고, 매니저 내부 재조회가 줄어들어 정합성과 테스트 용이성이 올라갑니다.src/main/java/konkuk/thip/roompost/application/service/VoteCreateService.java (1)
51-54: RoomParticipant 사전 조회 + 도메인 객체 기반 진행률 업데이트 — OK
roomParticipantCommandPort.getByUserIdAndRoomIdOrThrow(...)로 참여자 도메인을 확보한 뒤,roomProgressManager.updateUserAndRoomProgress(roomParticipant, room, book, vote.getPage())로 진행률을 갱신하는 흐름이RecordCreateService와 일관되고 깔끔합니다.Also applies to: 69-70
src/main/java/konkuk/thip/roompost/application/service/manager/RoomProgressManager.java (1)
20-37: 메서드 시그니처 변경과 평균 진행률 계산 로직 적합 — OK
- ID 기반에서 도메인 객체 기반 시그니처로 변경되어 응용 서비스에서의 책임이 명확해졌습니다.
- 현재 사용자 퍼센티지를 업데이트한 뒤, “타 참여자 합 + 현재 사용자 퍼센티지” / “전체 인원수”로 평균을 계산하는 방식이 의도와 부합합니다.
#️⃣ 연관된 이슈
📝 작업 내용
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신기능
문서
테스트