Conversation
Walkthrough방장 ID를 세션에 저장하고(룸 생성/입장 시), 방 활성 플레이어 수 조회 API를 추가하며 WebSocket 호스트 변경 브로드캐스트와 클라이언트의 시작 버튼/시작 흐름을 호스트 전용으로 변경합니다. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,235,255,0.6)
participant Client as 브라우저 (room.jsp / room.js)
participant View as ViewRoomController
participant Session as HTTP Session
participant DAO as RoomDAO
participant DB as 데이터베이스
end
Client->>View: GET /room?roomId=xxx
View->>Session: getSession(false) / getAttribute("loginUserId")
View->>DAO: findHostUserIdByRoomId(roomId)
DAO->>DB: SELECT host_user_id FROM room WHERE id = ?
DB-->>DAO: hostUserId
DAO-->>View: hostUserId
View->>Session: setAttribute("hostUserId", hostUserId)
View->>Session: setAttribute("userId", loginUserId)
View-->>Client: room.jsp (렌더링, data-user-id 포함)
sequenceDiagram
rect rgba(235,255,220,0.6)
participant WS as RoomWebSocket
participant Service as RoomWebSocketService
participant DAO as RoomDAO
participant DB as 데이터베이스
participant Clients as 모든 클라이언트
end
WS->>Service: onClose(session, roomId)
Service->>Service: exitAndHandleHost(roomId) -> result
alt 호스트 퇴장 -> HOST_CHANGE
Service->>DAO: findHostUserIdByRoomId(roomId)
DAO->>DB: SELECT host_user_id FROM room WHERE id = ?
DB-->>DAO: newHostUserId
DAO-->>Service: newHostUserId
Service->>Service: broadcastHostChanged(roomId, newHostUserId)
Service->>Clients: {type: "HOST_CHANGE", roomId, hostUserId}
else 일반 퇴장
Service->>Clients: {type: "USER_EXIT", ...}
end
sequenceDiagram
rect rgba(255,240,220,0.6)
participant Client as 브라우저 (room.js)
participant API as RoomController (/room/count)
participant DAO as RoomPlayerDAO
participant DB as 데이터베이스
end
Client->>API: GET /room/count?roomId=xxx
API->>API: 세션에서 loginUserId 검증
API->>DAO: countActivePlayers(roomId)
DAO->>DB: SELECT COUNT(*) ...
DB-->>DAO: activeCount
DAO-->>API: activeCount
API-->>Client: 200 {roomId, activeCount}
Client->>Client: playType 기준 최소 인원 검사
alt 충분 인원
Client->>Server: POST /game/start {roomId, playType}
Note over Client: WebSocket GAME_START 이벤트로 게임 진입 대기
else 부족
Client->>User: 경고 표시
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/game/controller/GameStartController.java (1)
49-67: 게임 시작 전에 플레이어 상태 업데이트 결과를 검증하세요.
updatePlayersToInGame(roomId)(63줄)의 반환값을 검증하지 않고 게임을 시작하고 있습니다. 반환값이 0이면 실제로 플레이어의 상태가 변경되지 않았는데도 브로드캐스트가 진행됩니다. 게임 시작 전에updated > 0인지 확인하고, 그렇지 않으면 오류를 반환해야 합니다.또한, 이미 구현된
roomPlayerDao.countActivePlayers(roomId)메서드를 활용하여 활성 플레이어 수를 먼저 검증하는 것을 권장합니다. ChatServer의 최소 플레이어 요구사항(GAME_START_PLAYERS = 2)과 일관성을 유지하세요.추가로, 56줄과 65줄의
System.out.println()디버그 문을 적절한 로깅 프레임워크로 대체하세요.
🧹 Nitpick comments (11)
src/main/java/game/controller/GameStartController.java (3)
24-26: Consider dependency injection for DAO instances.현재 DAO 인스턴스를 필드에서 직접 생성하면 테스트 시 mocking이 어렵고 느슨한 결합이 어려워집니다. 가능하다면 생성자 또는 setter를 통한 의존성 주입을 사용하는 것이 좋습니다.
56-56:System.out.println대신 logging framework 사용하기.프로덕션 코드에서는
System.out.println대신 SLF4J나 Log4j 같은 적절한 logging framework를 사용하는 것이 권장됩니다. 특히 line 56의 디버그 문은 의미가 명확하지 않으니 제거하거나 명확한 로그 메시지로 대체해 주세요.🔎 제안된 로깅 코드 예시
+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class GameStartController extends HttpServlet { private static final long serialVersionUID = 1L; + private static final Logger logger = LoggerFactory.getLogger(GameStartController.class); private final RoomDAO roomDao = new RoomDAOImpl(); private final RoomPlayerDAO roomPlayerDao = new RoomPlayerDAOImpl(); private final RoomWebSocketService service = new RoomWebSocketService();그리고 각 print 문을 다음과 같이 대체:
- System.out.println(hostUserId + " " + loginUserId); + logger.debug("Host verification - hostUserId: {}, loginUserId: {}", hostUserId, loginUserId);- System.out.println("[GameStart] roomId=" + roomId + " updatedPlayers=" + updated); + logger.info("Game started - roomId: {}, updatedPlayers: {}", roomId, updated);Also applies to: 65-65
74-77: 개선: 더 구체적인 예외 처리 추가하기.현재
Exception을 광범위하게 catch하고 있어서, 어떤 작업에서 오류가 발생했는지 구별하기 어렵습니다. 데이터베이스 오류와 다른 오류를 구분하여 처리하면 디버깅과 모니터링이 더 수월할 것입니다.🔎 예외 처리 개선 예시
} catch (Exception e) { + logger.error("Game start processing error - roomId: {}", roomId, e); e.printStackTrace(); response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "게임 시작 처리 중 오류가 발생했습니다."); }또는 더 구체적으로:
try { String hostUserId = roomDao.getHostUserId(roomId); // ... 나머지 코드 ... - } catch (Exception e) { + } catch (IllegalArgumentException e) { + logger.warn("Invalid game start request - roomId: {}", roomId, e); + response.sendError(HttpServletResponse.SC_BAD_REQUEST, "잘못된 게임 시작 요청입니다."); + } catch (Exception e) { + logger.error("Database error during game start - roomId: {}", roomId, e); e.printStackTrace(); response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "게임 시작 처리 중 오류가 발생했습니다."); }src/main/java/room/dao/RoomDAO.java (1)
25-30: 중복 메서드가 추가되었습니다.
findHostUserIdByRoomId(String roomId)는 기존getHostUserId(String roomId)(Line 23)와 동일한 기능을 수행합니다. 두 메서드 모두 roomId로 host_user_id를 조회하고 없으면 null을 반환합니다.기존
getHostUserId메서드를 재사용하거나, 하나로 통합하는 것을 권장합니다.src/main/java/lobby/controller/CreateRoomController.java (1)
41-43: 디버그용 System.out.println을 제거하거나 로거로 교체하세요.프로덕션 코드에 디버그용
System.out.println이 남아있습니다. 또한, 라인 42-43의session == null체크는 이미 라인 29-32에서 null 검증 후 반환하므로 불필요합니다.로깅이 필요하다면 SLF4J나 Log4j 같은 로깅 프레임워크를 사용하세요.
src/main/webapp/WEB-INF/views/room.jsp (1)
56-60: 버튼 타입을button으로 변경하세요.
<button type="submit">이지만 감싸는<form>이 없습니다. JavaScript 클릭 핸들러로 동작하므로type="button"이 의미상 더 적절합니다.🔎 수정 제안
<c:if test="${sessionScope.loginUserId eq sessionScope.hostUserId}"> - <button type="submit" id="btn-start" class="btn-start"> + <button type="button" id="btn-start" class="btn-start"> 🎯 시작하기 </button> </c:if>src/main/java/room/dao/RoomDAOImpl.java (1)
172-193: 중복 구현입니다.
findHostUserIdByRoomId메서드는 기존getHostUserId(Lines 149-170)와 완전히 동일한 로직입니다. 동일한 SQL 쿼리와 동일한 처리 패턴을 사용합니다.기존
getHostUserId메서드를 재사용하거나, 인터페이스에서 하나의 메서드로 통합하는 것을 권장합니다.🔎 통합 예시
-@Override -public String findHostUserIdByRoomId(String roomId) throws Exception { - - String sql = """ - SELECT host_user_id - FROM room - WHERE id = ? - """; - - try ( - Connection conn = DB.getConnection(); - PreparedStatement ps = conn.prepareStatement(sql)) { - ps.setString(1, roomId); - - try (ResultSet rs = ps.executeQuery()) { - if (rs.next()) { - return rs.getString("host_user_id"); - } - return null; - } - } -} +@Override +public String findHostUserIdByRoomId(String roomId) throws Exception { + return getHostUserId(roomId); +}src/main/webapp/static/room/room.js (2)
253-257: 불필요한 async 래퍼를 제거하세요.async arrow function이 단순히
startButtonClick()을 호출하기만 합니다. 직접 바인딩하는 것이 더 간결합니다.🔎 수정 제안
const btnStart = document.querySelector("#btn-start"); - - btnStart?.addEventListener("click", async () => { - startButtonClick() - }); +btnStart?.addEventListener("click", startButtonClick);
259-263: 파일 상단에 정의된 상수를 재사용하세요.
startButtonClick함수에서page,roomId,playType을 다시 쿼리하고 있습니다. 라인 2-6에서 이미 정의한pageEl,roomId,playType상수를 클로저로 활용할 수 있습니다.🔎 수정 제안
async function startButtonClick() { - const page = document.querySelector("#room-page"); - const roomId = page.dataset.roomId; - const playType = page.dataset.playType; - const contextPath = page.dataset.contextPath || ""; + const contextPath = pageEl?.dataset?.contextPath || ""; try { const countRes = await fetch(src/main/java/room/controller/RoomController.java (1)
40-68: 예외 처리 시 로깅 개선 권장
e.printStackTrace()대신 로깅 프레임워크 사용을 권장합니다. 프로덕션 환경에서 표준 에러 출력은 로그 수집과 모니터링에 적합하지 않습니다.🔎 SLF4J 로거 사용 예시
+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + @WebServlet("/room/*") public class RoomController extends HttpServlet { + private static final Logger log = LoggerFactory.getLogger(RoomController.class); // ... } catch (Exception e) { - e.printStackTrace(); + log.error("Handler execution failed for action: {}", action, e); sendJson(res, 500, ApiError.of("서버 오류가 발생했습니다.")); }src/main/java/room/ws/RoomWebSocketService.java (1)
86-95: 예외 발생 시 조용한 실패HOST_CHANGE 처리 중 예외가 발생하면 스택 트레이스만 출력하고 클라이언트에 알림 없이 진행됩니다. 새 방장에게 시작 버튼이 표시되지 않을 수 있습니다. 최소한 로그 레벨을 높이거나, 재시도 로직을 고려해보세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/game/controller/GameStartController.javasrc/main/java/lobby/controller/CreateRoomController.javasrc/main/java/room/controller/RoomController.javasrc/main/java/room/controller/ViewRoomController.javasrc/main/java/room/dao/RoomDAO.javasrc/main/java/room/dao/RoomDAOImpl.javasrc/main/java/room/dto/RoomActiveCountDTO.javasrc/main/java/room/ws/RoomWebSocket.javasrc/main/java/room/ws/RoomWebSocketService.javasrc/main/webapp/WEB-INF/views/lobby.jspsrc/main/webapp/WEB-INF/views/room.jspsrc/main/webapp/static/room/room.js
💤 Files with no reviewable changes (1)
- src/main/webapp/WEB-INF/views/lobby.jsp
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/room/dao/RoomDAO.java (1)
src/main/webapp/static/room/room.js (1)
roomId(3-3)
src/main/java/room/dto/RoomActiveCountDTO.java (1)
src/main/webapp/static/room/room.js (1)
roomId(3-3)
src/main/java/room/dao/RoomDAOImpl.java (1)
src/main/java/util/DB.java (1)
DB(8-23)
src/main/java/room/ws/RoomWebSocketService.java (1)
src/main/java/room/dao/RoomDAOImpl.java (1)
RoomDAOImpl(15-206)
🔇 Additional comments (15)
src/main/java/game/controller/GameStartController.java (1)
62-62: 코드 가독성 개선 승인.호스트 검증 블록과 플레이어 업데이트 로직 사이의 공백 라인 추가로 논리적 섹션이 명확히 구분되어 가독성이 향상되었습니다.
src/main/java/lobby/controller/CreateRoomController.java (1)
63-64: LGTM!방 생성 시 세션에 hostUserId를 저장하여 방장 정보를 JSP에서 활용할 수 있도록 한 점이 적절합니다.
src/main/webapp/WEB-INF/views/room.jsp (1)
19-19: LGTM!
data-user-id속성 추가로 JavaScript에서 현재 사용자 ID를 참조할 수 있게 되었습니다.src/main/java/room/dto/RoomActiveCountDTO.java (1)
1-11: LGTM!간결하고 적절한 DTO 구현입니다. Lombok 어노테이션을 활용하여 보일러플레이트 코드를 최소화했습니다.
src/main/java/room/ws/RoomWebSocket.java (2)
148-157: LGTM!
onExit호출 시 exit 타입("ROOM_EXIT")을 전달하여 서비스 레이어에서 적절한 처리가 가능하도록 했습니다.
214-219: LGTM!
exitAndHandleHost의 결과값을onExit에 전달하여 방장 변경 시 HOST_CHANGE 이벤트 브로드캐스트가 가능하도록 한 구현이 적절합니다.src/main/webapp/static/room/room.js (1)
187-207: HOST_CHANGE 핸들러 구현이 적절합니다.방장 변경 시 새 방장에게 시작 버튼을 동적으로 생성하는 로직이 잘 구현되었습니다. 기존 버튼 존재 여부 확인과 중복 생성 방지 처리도 포함되어 있습니다.
src/main/java/room/controller/RoomController.java (4)
1-31: 서블릿 구조 및 의존성 LGTM서블릿 구조가 깔끔합니다.
@WebServlet("/room/*")어노테이션과 함수형 Handler 인터페이스 패턴이 적절히 사용되었습니다.
74-74: 사용되지 않는 userId 파라미터
handleGetCount메서드에서userId파라미터가 전달되지만 사용되지 않습니다. 향후 권한 검증(예: 해당 방의 참여자인지 확인)에 활용할 예정이라면 현재 상태는 괜찮습니다. 그렇지 않다면 Handler 인터페이스 설계를 재검토하거나, 이 엔드포인트에서 인증 검증을 건너뛰는 것을 고려해보세요.
85-90: 경로 파싱 로직 LGTM
getAction메서드가 pathInfo를 적절히 처리하고 있습니다. null 체크와 루트 경로 처리가 올바릅니다.
103-143: 내부 응답 클래스 구조 적절
ApiSuccess와ApiError클래스가 일관된 API 응답 형식을 제공합니다. Gson이 리플렉션을 통해 필드를 직렬화하므로 getter 메서드는 필수는 아니지만, 향후 확장성을 위해 유지해도 무방합니다.src/main/java/room/ws/RoomWebSocketService.java (4)
12-13: DAO 의존성 추가 LGTM방장 정보 조회를 위한
RoomDAO의존성 추가가 적절합니다.Also applies to: 30-30
106-106: cleanup에서 null result 전달 - 의도 확인
cleanup호출 시result에 null을 전달합니다. WebSocket 연결 종료 시에도 방장 변경이 발생할 수 있는데, 이 경우 HOST_CHANGE 이벤트가 전파되지 않습니다. 방장이 브라우저를 닫거나 네트워크 연결이 끊어지는 시나리오에서 의도한 동작인지 확인이 필요합니다.
128-137: broadcastHostChanged 메서드 구현 적절null/blank 체크 및 페이로드 구성이 기존
broadcastGameStart패턴과 일관됩니다.
80-80: 호환성 확인 완료 - 모든 호출부가 이미 업데이트됨
onExit메서드의 3개 파라미터 시그니처로의 변경은 이미 모든 호출부에서 처리되어 있습니다. RoomWebSocket.java의 모든onExit호출(151줄, 218줄)이 새 시그니처(Session, String, String)를 사용하고 있으므로 컴파일 에러가 발생하지 않습니다.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/main/webapp/static/room/room.js (2)
186-206: 들여쓰기 일관성 문제와 null 검증 추가 권장다음 사항들을 개선해 주세요:
- 들여쓰기 불일치: 이 섹션은 탭을 사용하지만, 파일의 나머지 부분은 스페이스를 사용합니다. 일관성을 위해 스페이스로 통일해 주세요.
- null 안전성: Line 191에서
p.hostUserId를 비교하기 전에 존재 여부를 검증하는 것이 좋습니다.🔎 개선 제안
- - case "HOST_CHANGE": { - appendSystemLog("방장이 변경되었습니다."); - const p = msg.payload; - - if (p.hostUserId !== userId) break; - - if (document.querySelector("#btn-start")) break; - - const btn = document.createElement("button"); - btn.type = "button"; - btn.id = "btn-start"; - btn.className = "btn-start"; - btn.textContent = "🎯 시작하기"; - - btn.addEventListener("click", startButtonClick); - - document.querySelector(".side-nav")?.appendChild(btn); - break; - - } + + case "HOST_CHANGE": { + appendSystemLog("방장이 변경되었습니다."); + const p = msg.payload; + + if (!p.hostUserId || p.hostUserId !== userId) break; + + if (document.querySelector("#btn-start")) break; + + const btn = document.createElement("button"); + btn.type = "button"; + btn.id = "btn-start"; + btn.className = "btn-start"; + btn.textContent = "🎯 시작하기"; + + btn.addEventListener("click", startButtonClick); + + document.querySelector(".side-nav")?.appendChild(btn); + break; + }
252-305: Optional chaining 추가 및 코드 간결화 권장다음 사항들을 개선해 주세요:
- Optional chaining 누락 (Line 259-262):
page.dataset에 접근할 때 optional chaining을 사용하지 않아page가 null이면 TypeError가 발생할 수 있습니다.- 불필요한 wrapper 함수 (Line 254-256): 화살표 함수로 감싸지 않고
startButtonClick을 직접 전달할 수 있습니다.- 들여쓰기 불일치: 전체적으로 탭 대신 스페이스를 사용해 주세요.
🔎 개선 제안
const btnStart = document.querySelector("#btn-start"); - btnStart?.addEventListener("click", async () => { - startButtonClick() - }); - - async function startButtonClick() { - const page = document.querySelector("#room-page"); - const roomId = page.dataset.roomId; - const playType = page.dataset.playType; - const contextPath = page.dataset.contextPath || ""; + btnStart?.addEventListener("click", startButtonClick); + + async function startButtonClick() { + const page = document.querySelector("#room-page"); + const roomId = page?.dataset?.roomId; + const playType = page?.dataset?.playType; + const contextPath = page?.dataset?.contextPath || ""; + + if (!roomId || !playType) { + alert("방 정보를 가져올 수 없습니다."); + return; + }나머지 함수 본문도 동일하게 스페이스로 들여쓰기를 변경해 주세요.
src/main/java/room/controller/ViewRoomController.java (1)
18-18: 의존성 주입 고려 권장
RoomDAO를 직접 인스턴스화하고 있습니다. 향후 테스트 용이성과 결합도 감소를 위해 의존성 주입 패턴 도입을 고려해 보세요.src/main/java/room/ws/RoomWebSocketService.java (2)
30-30: 의존성 주입 고려 권장
RoomDAO를 직접 인스턴스화하고 있습니다. 향후 테스트 용이성과 결합도 감소를 위해 의존성 주입 패턴 도입을 고려해 보세요.
86-97: null 체크 잘 구현됨, 로깅 개선 권장이전 리뷰의 null hostUserId 처리 문제가 Line 90-92의 null 체크로 잘 해결되었습니다. 다만 Line 94의
printStackTrace()는 프로덕션 환경에 적합하지 않으므로 적절한 로거(예: SLF4J, Log4j 등)를 사용해 주세요.🔎 로깅 개선 제안
if ("HOST_CHANGE".equals(result)) { String hostUserId; try { hostUserId = roomDao.getHostUserId(roomId); if (hostUserId != null) { broadcastHostChanged(roomId, hostUserId); } } catch (Exception e) { - e.printStackTrace(); + // TODO: Replace with proper logger (e.g., SLF4J) + System.err.println("[RoomWS] Failed to get hostUserId: " + e.getMessage()); } }더 나은 방법은 로깅 프레임워크를 도입하는 것입니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/room/controller/ViewRoomController.javasrc/main/java/room/ws/RoomWebSocketService.javasrc/main/webapp/static/room/room.js
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/room/ws/RoomWebSocketService.java (1)
src/main/java/room/dao/RoomDAOImpl.java (1)
RoomDAOImpl(15-206)
src/main/java/room/controller/ViewRoomController.java (3)
src/main/java/room/dao/RoomDAOImpl.java (1)
RoomDAOImpl(15-206)src/main/java/room/controller/EnterRoomController.java (1)
WebServlet(18-58)src/main/webapp/static/room/room.js (2)
roomId(3-3)userId(5-5)
🔇 Additional comments (2)
src/main/webapp/static/room/room.js (1)
5-5: LGTM!userId 추출 로직이 올바르게 구현되었으며, optional chaining을 일관성 있게 사용하고 있습니다.
src/main/java/room/ws/RoomWebSocketService.java (1)
130-139: LGTM!
broadcastHostChanged메서드가 올바르게 구현되었습니다. 입력 검증과 페이로드 구성이 적절하며, 기존 broadcast 패턴과 일관성 있게 작성되었습니다.
3 participants
📌 작업 내용
게임 시작 버튼 표시 및 게임 시작 요청 시점에 방장 여부와 방 인원상태를 검증하고, 방장 변경 상황을 WebSocket으로 실시간 반영하도록 개선했습니다.
🔗 관련 이슈
👀 리뷰 시 참고사항
💭 느낀 점
💻 스크린샷 (선택)
Summary by CodeRabbit
새로운 기능
개선 사항
버그 픽스 / 오류처리
✏️ Tip: You can customize this high-level summary in your review settings.