feat :: [#102] 합주방 상세조회, 포지션 설정 #103
Conversation
|
""" Walkthrough이 변경사항은 합주방 상세 조회 API와 합주방 내 포지션 선택 메시지 처리를 추가합니다. 새로운 Room 상세 조회 엔드포인트가 RoomController에 추가되었으며, GetRoomDetailUseCase와 관련 DTO가 도입되었습니다. Member 엔티티에 position 필드가 추가되었고, UpdateMemberPositionUseCase가 새로 생성되어 멤버의 포지션을 갱신할 수 있게 되었습니다. RoomWebSocketHandler에서는 "position" 신호 타입을 처리하여 포지션 변경을 실시간으로 브로드캐스트합니다. 기존 기능은 변경되지 않았으며, 새로운 기능이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RoomController
participant GetRoomDetailUseCase
participant QueryRoomService
participant CheckRoomService
participant QueryUserService
Client->>RoomController: GET /rooms/{room-id}
RoomController->>GetRoomDetailUseCase: execute(roomId)
GetRoomDetailUseCase->>QueryUserService: getCurrentUser()
GetRoomDetailUseCase->>QueryRoomService: getRoomById(roomId)
GetRoomDetailUseCase->>CheckRoomService: checkMember(room, user)
GetRoomDetailUseCase->>QueryRoomService: getMembersByRoomOrderByJoinedAt(roomId)
GetRoomDetailUseCase-->>RoomController: GetRoomDetailResponse
RoomController-->>Client: 200 OK + RoomDetail
sequenceDiagram
participant Client
participant RoomWebSocketHandler
participant UpdateMemberPositionUseCase
participant QueryRoomService
participant QueryUserService
Client->>RoomWebSocketHandler: "position" Signal
RoomWebSocketHandler->>UpdateMemberPositionUseCase: execute(roomId, userEmail, position)
UpdateMemberPositionUseCase->>QueryRoomService: getRoomById(roomId)
UpdateMemberPositionUseCase->>QueryUserService: getUserByEmail(userEmail)
UpdateMemberPositionUseCase->>QueryRoomService: getMemberByUserEmailAndRoomId(userEmail, roomId)
UpdateMemberPositionUseCase-->>RoomWebSocketHandler: (void)
RoomWebSocketHandler-->>Client: Broadcast "position" Signal
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
""" Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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. 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: 8
🧹 Nitpick comments (2)
src/main/kotlin/dsm/wemeet/domain/room/repository/model/Member.kt (1)
36-38: 합주방 멤버 포지션 필드 추가Position 필드를 nullable로 정의하고 문자열 형태로 저장하도록 설정한 것이 적절합니다. var로 선언하여 포지션 변경이 가능하도록 한 점도 좋습니다.
다만, 필드에 대한 간단한 주석을 추가하면 다른 개발자들이 이 필드의 용도를 더 쉽게 이해할 수 있을 것입니다.
@Enumerated(EnumType.STRING) @Column(nullable = true, name = "position") +// 합주방 내 멤버의 악기/포지션 정보 var position: Position? = nullsrc/main/kotlin/dsm/wemeet/domain/room/presentation/RoomController.kt (1)
85-89: 합주방 상세 조회 엔드포인트 추가합주방 ID를 경로 변수로 받아 상세 정보를 조회하는 엔드포인트를 적절히 구현했습니다.
다만, 한 가지 제안할 내용은 메서드 이름과 경로 변수명의 일관성을 고려해보는 것입니다. 메서드 이름은
getRoomDetails(복수형)인데 반환 타입은GetRoomDetailResponse(단수형)입니다. 일관성을 위해 메서드 이름을getRoomDetail로 변경하는 것을 고려해보세요.@ResponseStatus(HttpStatus.OK) @GetMapping("/{room-id}") -fun getRoomDetails(@PathVariable("room-id") roomId: UUID): GetRoomDetailResponse { +fun getRoomDetail(@PathVariable("room-id") roomId: UUID): GetRoomDetailResponse { return getRoomDetailUseCase.execute(roomId) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/kotlin/dsm/wemeet/domain/room/presentation/RoomController.kt(3 hunks)src/main/kotlin/dsm/wemeet/domain/room/presentation/dto/response/GetRoomDetailResponse.kt(1 hunks)src/main/kotlin/dsm/wemeet/domain/room/repository/model/Member.kt(2 hunks)src/main/kotlin/dsm/wemeet/domain/room/usecase/GetRoomDetailUseCase.kt(1 hunks)src/main/kotlin/dsm/wemeet/domain/room/usecase/UpdateMemberPositionUseCase.kt(1 hunks)src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt(6 hunks)
🔇 Additional comments (8)
src/main/kotlin/dsm/wemeet/domain/room/repository/model/Member.kt (1)
3-3: 적절한 import 추가Position 열거형과 EnumType 관련 import를 추가하여 새로운 필드의 타입과 저장 방식을 정의했습니다.
Also applies to: 7-8
src/main/kotlin/dsm/wemeet/domain/room/presentation/RoomController.kt (2)
6-6: 필요한 import 추가새로운 기능에 필요한 DTO와 UseCase import를 추가했습니다.
Also applies to: 10-10
36-36: UseCase 의존성 주입GetRoomDetailUseCase 의존성을 컨트롤러에 주입하여 새로운 기능을 구현할 준비가 되었습니다.
src/main/kotlin/dsm/wemeet/domain/room/presentation/dto/response/GetRoomDetailResponse.kt (1)
5-12: 합주방 상세 정보 응답 DTO 정의합주방의 기본 정보와 멤버 목록을 포함하는 응답 DTO를 잘 구성했습니다.
src/main/kotlin/dsm/wemeet/domain/room/usecase/GetRoomDetailUseCase.kt (2)
11-16: 적절한 의존성 주입필요한 서비스들을 의존성으로 주입받아 기능 구현을 준비했습니다.
18-23: 사용자 권한 검증 로직현재 로그인한 사용자가 요청한 합주방의 멤버인지 확인하는 검증 로직이 잘 구현되어 있습니다.
src/main/kotlin/dsm/wemeet/domain/room/usecase/UpdateMemberPositionUseCase.kt (1)
11-12: 트랜잭션 범위 확인 필요이 유스케이스는
@Transactional어노테이션으로 트랜잭션 처리되고 있습니다. 이로 인해 엔티티 변경사항이 트랜잭션 커밋 시점에 자동으로 저장되는데, 이것이 의도된 동작인지 확인해보세요. 현재 코드에는 명시적인save()호출이 없습니다.src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt (1)
30-31: 의존성 추가: 정상적으로 구성됨
updateMemberPositionUseCase의존성이 적절하게 주입되었습니다. 이 의존성은 포지션 업데이트 기능에 필요하며 올바르게 구성되었습니다.
src/main/kotlin/dsm/wemeet/domain/room/presentation/dto/response/GetRoomDetailResponse.kt
Show resolved
Hide resolved
src/main/kotlin/dsm/wemeet/domain/room/usecase/UpdateMemberPositionUseCase.kt
Show resolved
Hide resolved
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt
Outdated
Show resolved
Hide resolved
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 (1)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt(5 hunks)
🔇 Additional comments (4)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt (4)
9-10: 의존성 추가가 적절합니다.포지션 업데이트 기능에 필요한 새로운 의존성을 추가했습니다.
UpdateMemberPositionUseCase와Position열거형을 가져와 소켓 핸들러에서 멤버 포지션 업데이트 기능을 구현하는데 사용합니다.
33-34: 의존성 주입이 적절합니다.
checkIsMemberUseCase와updateMemberPositionUseCase를 적절하게 생성자 주입으로 추가했습니다. 이는 의존성 주입 패턴을 일관되게 유지하며 멤버 포지션 업데이트 기능을 구현하는데 필요합니다.
66-66: 세션 상태 검증이 적절합니다.세션이 열려있는지 확인한 후 메시지를 보내도록 수정되었습니다. 이는 이전에
takeIf/let패턴에서 발생했던 문제를 해결하는 올바른 접근법입니다.
99-101: 세션 상태 검증이 올바르게 구현되었습니다.
?.takeIf { it.isOpen }패턴을 사용하여 세션이 열려있는 경우에만 메시지를 전송하도록 수정했습니다. 이전 코드에서 발생했던 세션 검증 문제가 해결되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt (1)
121-146: 🛠️ Refactor suggestion포지션 변경 기능 구현 개선 필요
포지션 변경 로직에 몇 가지 개선이 필요합니다:
- 입력값 검증: "position" 필드가 실제로 존재하는지 확인하는 검증이 없습니다.
- 예외 정보 로깅: 예외가 발생할 경우 로그를 남기지 않아 디버깅이 어렵습니다.
- 메시지 구성: 클라이언트에 오류 피드백이 제공되지 않습니다.
"position" -> { signal.data?.let { + if (!it.has("position") || it.get("position").isNull) { + logger.debug("position 필드가 없거나 null입니다") + return + } val position = try { Position.valueOf(it.get("position").asText()) } catch (e: Exception) { + logger.debug("잘못된 position 값: {}", it.get("position").asText()) + if (session.isOpen) { + val errorMsg = createMsg("error", "유효하지 않은 포지션 값입니다") + session.sendMessage(TextMessage(errorMsg.toString())) + } return } val mail = getUserEmail(session) updateMemberPositionUseCase.execute(roomId, mail, position) val positionMsg = Signal( type = signal.type, to = null, data = objectMapper.createObjectNode().apply { put("mail", mail) put("position", position.name) } ) peers.forEach { peer -> if (peer.isOpen && peer.attributes["email"]!!.toString() != mail) peer.sendMessage(TextMessage(objectMapper.writeValueAsString(positionMsg))) } } }🧰 Tools
🪛 detekt (1.23.7)
[warning] 126-126: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🧹 Nitpick comments (3)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt (3)
92-96: 예외 처리 향상 필요메시지 파싱 시 발생하는 예외를 무시하고 있습니다. 디버깅을 위해 최소한 로그는 남기는 것이 좋습니다.
try { objectMapper.readValue(message.payload, Signal::class.java) } catch (e: Exception) { + logger.debug("메시지 파싱 실패: {}", e.message) return }🧰 Tools
🪛 detekt (1.23.7)
[warning] 94-94: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
103-105: 세션 상태 검증 개선타겟 피어에게 메시지를 보내기 전에 세션이 열려있는지 확인하는 것은 적절합니다. 하지만
takeIf와 함께 체이닝된 함수 호출은 가독성 측면에서if (it.isOpen)조건문으로 대체하는 것이 더 명확할 수 있습니다.peers.find { it.attributes["email"] == targetEmail } - ?.takeIf { it.isOpen } - ?.sendMessage(TextMessage(objectMapper.writeValueAsString(signal))) + ?.let { + if (it.isOpen) { + it.sendMessage(TextMessage(objectMapper.writeValueAsString(signal))) + } + }
142-144: 피어 검증 및 메시지 전송 코드는 좋습니다.메시지를 브로드캐스트할 때 세션이 열려있는지 확인하고 발신자를 제외하는 로직이 잘 구현되어 있습니다. 다만, 가독성을 위해 라인을 분리하는 것이 좋을 수 있습니다.
peers.forEach { peer -> - if (peer.isOpen && peer.attributes["email"]!!.toString() != mail) peer.sendMessage(TextMessage(objectMapper.writeValueAsString(positionMsg))) + if (peer.isOpen && peer.attributes["email"]!!.toString() != mail) { + peer.sendMessage(TextMessage(objectMapper.writeValueAsString(positionMsg))) + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt (1)
Learnt from: zios0707
PR: BremenMusicians/WeMeet_BE#103
File: src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt:117-138
Timestamp: 2025-05-08T02:31:56.008Z
Learning: Kotlin에서 `return@let`은 `let` 블록만 종료하고 외부 코드 흐름을 계속 진행하는 반면, `return`은 전체 함수를 종료한다. 이는 람다 함수 내에서 로컬 반환(local return)과 비로컬 반환(non-local return)의 차이이다.
🪛 detekt (1.23.7)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt
[warning] 94-94: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 126-126: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (3)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt (3)
9-10: 새로운 의존성 및 Position 열거형 추가
UpdateMemberPositionUseCase와Position열거형을 적절하게 임포트하여 포지션 관련 기능을 구현할 준비가 되었습니다.
33-34: 필요한 의존성 주입이 잘 구현되었습니다.새로운 기능 구현에 필요한
UpdateMemberPositionUseCase가 적절하게 의존성으로 추가되었습니다.
66-66: 세션 상태 확인 추가세션이 열려있는지 확인 후 메시지를 전송하는 안전장치가 추가되었습니다. 이는 예외 상황을 방지하는 좋은 방어 코드입니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt (2)
92-96: 예외 처리가 추가되었지만 로깅이 없습니다.시그널 파싱에 예외 처리가 추가된 것은 좋지만, 예외를 단순히 무시하고 있습니다. 디버깅을 위해 로깅을 추가하는 것이 도움이 될 수 있습니다.
try { objectMapper.readValue(message.payload, Signal::class.java) } catch (e: Exception) { + logger.debug("메시지 파싱 오류: ${e.message}") return }🧰 Tools
🪛 detekt (1.23.7)
[warning] 94-94: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
121-148: 포지션 업데이트 기능 구현에 대한 몇 가지 개선점이 있습니다.포지션 업데이트 기능이 잘 구현되었습니다만, 다음과 같은 개선점이 있습니다:
- JSON에서 "position" 필드의 존재 여부를 확인하는 검증 로직이 없습니다.
- 예외를 단순히 무시하고 있어 디버깅이 어려울 수 있습니다.
- 144번 줄에서
!!연산자를 사용하여 이메일을 강제 언래핑하고 있습니다. 안전한 접근 방식으로 변경하는 것이 좋습니다.signal.data?.let { + if (!it.has("position") || it.get("position").isNull) { + logger.debug("유효하지 않은 포지션 데이터") + return + } val position = try { Position.valueOf(it.get("position").asText()) } catch (e: Exception) { + logger.debug("포지션 파싱 오류: ${e.message}") return } // ...생략... peers.forEach { peer -> - if (peer.isOpen && peer.attributes["email"]!!.toString() != mail) { + if (peer.isOpen && peer.attributes["email"]?.toString() != mail) { peer.sendMessage(TextMessage(objectMapper.writeValueAsString(positionMsg))) } } }🧰 Tools
🪛 detekt (1.23.7)
[warning] 126-126: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt (1)
Learnt from: zios0707
PR: BremenMusicians/WeMeet_BE#103
File: src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt:117-138
Timestamp: 2025-05-08T02:31:56.008Z
Learning: Kotlin에서 `return@let`은 `let` 블록만 종료하고 외부 코드 흐름을 계속 진행하는 반면, `return`은 전체 함수를 종료한다. 이는 람다 함수 내에서 로컬 반환(local return)과 비로컬 반환(non-local return)의 차이이다.
🪛 detekt (1.23.7)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt
[warning] 94-94: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 126-126: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (5)
src/main/kotlin/dsm/wemeet/global/socket/domain/RoomWebSocketHandler.kt (5)
9-10: 포지션 업데이트 기능 추가를 위한 필요한 임포트가 적절히 추가되었습니다.새로운 포지션 업데이트 기능을 위해 필요한
UpdateMemberPositionUseCase와Position열거형 클래스가 적절히 추가되었습니다.
33-34: 의존성 주입이 올바르게 구현되었습니다.
updateMemberPositionUseCase의존성이 생성자를 통해 올바르게 주입되었습니다.
66-66: 세션 상태 확인 로직이 적절히 추가되었습니다.세션이 열려있는지 확인 후 메시지를 전송하도록 안전하게 수정되었습니다. 이는 닫힌 세션에 메시지를 전송할 때 발생할 수 있는 예외를 방지하는 좋은 방법입니다.
103-104: 세션 상태 확인 로직이 적절히 추가되었습니다.WebRTC 메시지를 전송하기 전에 세션이 열려있는지 확인하는 로직이 적절히 추가되었습니다.
185-188: 메소드 매개변수 이름이 적절히 변경되었습니다.
createMsg메소드의 매개변수와 JSON 키가 "payload"에서 "data"로 변경되었습니다. 이는 명명 일관성을 위한 좋은 리팩토링입니다.
resolve #102
Summary by CodeRabbit