Conversation
요약Walkthrough채팅 시스템의 사용자 입장/퇴장 메시지 처리를 위한 인프라를 구축합니다. 세션 레지스트리, DTO, 서비스 메서드, 웹소켓 엔드포인트를 추가하고 테스트 UI를 구현하여 join/leave 기능을 지원합니다. Changes
Sequence DiagramsequenceDiagram
actor Client as 클라이언트
participant WS as STOMP<br/>WebSocket
participant Ctrl as ChatStompController
participant Reg as ChatSessionRegistry
participant Svc as ChatMessageService
participant DB as Database
participant Topic as /topic/chat/rooms/{roomId}
Client->>WS: join(roomId, userId, nickname)
WS->>Ctrl: `@MessageMapping` /app/chat.join
Ctrl->>Reg: put(sessionId, roomId, userId, nickname)
Reg->>Reg: 세션 저장
Ctrl->>Svc: saveJoinLeave(room, JOIN, userId, nickname, null)
Svc->>DB: 입장 메시지 저장
DB-->>Svc: ChatMessage
Svc-->>Ctrl: ChatMessage
Ctrl->>Topic: convertAndSend (입장 메시지)
Topic-->>Client: 모든 구독자에게 브로드캐스트
Client->>WS: leave(roomId, userId, nickname)
WS->>Ctrl: `@MessageMapping` /app/chat.leave
Ctrl->>Reg: remove(sessionId)
Reg->>Reg: 세션 삭제
Ctrl->>Svc: saveJoinLeave(room, LEAVE, userId, nickname, null)
Svc->>DB: 퇴장 메시지 저장
DB-->>Svc: ChatMessage
Svc-->>Ctrl: ChatMessage
Ctrl->>Topic: convertAndSend (퇴장 메시지)
Topic-->>Client: 모든 구독자에게 브로드캐스트
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 분 Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/main/java/com/be/sportizebe/domain/chat/service/ChatMessageService.java`:
- Around line 67-79: The saveSystem method in ChatMessageService appears to be
dead/ambiguous: either remove it or make its purpose explicit; either (A) delete
the unused saveSystem(ChatRoom, ChatMessage.Type, String) and any related tests
if no callers exist, or (B) restrict and document it to only create pure system
messages by changing the signature to accept only ChatMessage.Type.SYSTEM
(remove JOIN/LEAVE from allowed values), add a clear javadoc comment, and ensure
callers use saveJoinLeave for JOIN/LEAVE while routing true system notifications
to this method; update any references to use messageRepository.save(msg)
consistently and add a unit test demonstrating the intended behavior.
In
`@src/main/java/com/be/sportizebe/domain/chat/websocket/ChatSessionRegistry.java`:
- Around line 8-26: The registry currently never cleans up when sockets
disconnect; add a WebSocket disconnect event listener that calls
ChatSessionRegistry.remove(sessionId) to clear Presence entries and broadcast a
leave message: implement a component (e.g., WebSocketEventListener) with an
`@EventListener` method handleSessionDisconnect(SessionDisconnectEvent) that
retrieves the sessionId, removes the Presence from ChatSessionRegistry, fetches
the ChatRoom via ChatRoomService, creates/saves a LEAVE ChatMessage via
ChatMessageService (using presence.userId() and presence.nickname()), and sends
the ChatMessageResponse over SimpMessagingTemplate to
"/topic/chat/rooms/{roomId}" so abandoned sessions are removed and clients are
notified.
In
`@src/main/java/com/be/sportizebe/domain/chat/websocket/ChatStompController.java`:
- Around line 57-58: The join() and leave() handlers are broadcasting
ChatMessage entities directly (via messagingTemplate.convertAndSend) which leads
to inconsistent JSON vs send() (which uses ChatMessageResponse.from(saved)),
potential LazyInitializationException for room (FetchType.LAZY), and data
leakage; change join() and leave() to map the ChatMessage entity to the same DTO
used in send() by calling ChatMessageResponse.from(...) (or an equivalent
converter) and broadcast that DTO instead, ensuring any needed associations are
initialized inside the transaction/service layer before conversion so no lazy
fields are accessed during serialization.
- Around line 74-75: The leave() handler in ChatStompController is broadcasting
the raw entity instead of the DTO, causing the same direct-entity broadcast
issue; change the messagingTemplate.convertAndSend call in leave() to send
ChatMessageResponse.from(msg) (same pattern used elsewhere) so you convert the
domain object to the response DTO before broadcasting, updating the call that
currently constructs the destination "/topic/chat/rooms/" + req.getRoomId() and
uses msg.
In `@src/main/java/com/be/sportizebe/test/stomp-test.html`:
- Around line 433-468: In subscribeRoom(), validate the roomId and userId before
building the destination or sending join: call roomIdValue() and ensure it's
non-empty and Number(roomIdValue()) is not NaN, and call myUserId() and ensure
it's a finite number (not NaN); if either check fails, log a debug/warn and
return early instead of subscribing or sending the /app/chat.join payload. Also
ensure destination string construction uses the validated numeric roomId and
include these checks near the top of subscribeRoom (before dest/currentSub
logic) referencing subscribeRoom, roomIdValue(), myUserId(), stompClient,
currentSub and stompClient.send.
🧹 Nitpick comments (4)
src/main/java/com/be/sportizebe/domain/chat/dto/ChatPresenceRequest.java (1)
6-12: 입력 유효성 검증 추가를 권장합니다.
roomId,userId,nickname필드에@NotNull등의 유효성 검증 어노테이션이 없습니다. 잘못된 요청이 들어올 경우NullPointerException이 발생할 수 있습니다.♻️ 제안하는 수정
package com.be.sportizebe.domain.chat.dto; import lombok.Getter; import lombok.NoArgsConstructor; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.NotNull; `@Getter` `@NoArgsConstructor` public class ChatPresenceRequest { + `@NotNull` private Long roomId; + `@NotNull` private Long userId; + `@NotBlank` private String nickname; }src/main/java/com/be/sportizebe/domain/chat/websocket/ChatStompController.java (1)
39-59: 중복 입장 처리 고려동일 사용자가 이미 방에 입장한 상태에서 다시
join을 호출하면 중복 입장 메시지가 브로드캐스트됩니다. 기존 세션을 확인하거나, 중복 입장을 방지하는 로직을 추가하는 것을 고려하세요.♻️ 제안하는 수정
`@MessageMapping`("/chat.join") public void join(ChatPresenceRequest req, SimpMessageHeaderAccessor headerAccessor) { String sessionId = headerAccessor.getSessionId(); + // 이미 입장한 세션인지 확인 + if (registry.get(sessionId) != null) { + return; // 또는 로깅 + } + ChatRoom room = chatRoomService.getOrThrow(req.getRoomId()); // ...src/main/java/com/be/sportizebe/test/stomp-test.html (2)
304-314: 디버그 로그 무제한 누적 방지 고려장시간 테스트 시
debugLog가 계속 커져 DOM/메모리 부담이 생길 수 있습니다. 로그 라인 상한을 두는 편이 안전합니다.♻️ 제안 변경
- let lastSent = { text: "", at: 0 }; + let lastSent = { text: "", at: 0 }; + const MAX_LOG_LINES = 500; function debug(msg){ const el = $("debugLog"); - el.textContent += msg + "\n"; + const next = (el.textContent + msg + "\n").split("\n"); + if (next.length > MAX_LOG_LINES) { + el.textContent = next.slice(-MAX_LOG_LINES).join("\n"); + } else { + el.textContent += msg + "\n"; + } el.scrollTop = el.scrollHeight; }
471-498: DOM 조회 중복 제거로 일관성 확보이미
roomIdValue(),myUserId(),myNickname()가 있으니 재사용하면 로직 일관성과 유지보수성이 좋아집니다.♻️ 제안 변경
- const payload = { - roomId: Number(document.getElementById('roomId').value), - senderUserId: Number(document.getElementById('senderUserId').value), - senderNickname: document.getElementById('senderNickname').value.trim(), - content: text - }; + const payload = { + roomId: Number(roomIdValue()), + senderUserId: myUserId(), + senderNickname: myNickname(), + content: text + };
| // 채팅이 아닌 시스템 메시지 전용! | ||
| @Transactional | ||
| public ChatMessage saveSystem(ChatRoom room, ChatMessage.Type type, String content) { | ||
| ChatMessage msg = ChatMessage.builder() | ||
| .room(room) | ||
| .senderUserId(0L) | ||
| .senderNickname("SYSTEM") | ||
| .type(type) // JOIN / LEAVE / SYSTEM | ||
| .content(content) | ||
| .build(); | ||
|
|
||
| return messageRepository.save(msg); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for saveSystem method usage in Java files
rg -n "saveSystem" --type=java -B 2 -A 2Repository: Sportize/be
Length of output: 660
🏁 Script executed:
#!/bin/bash
# Read the ChatMessageService file to see both saveSystem and saveJoinLeave methods
cat -n src/main/java/com/be/sportizebe/domain/chat/service/ChatMessageService.java | sed -n '67,90p'Repository: Sportize/be
Length of output: 1095
saveSystem 메서드는 사용되지 않는 데드 코드입니다. 제거 또는 용도 명확히 필요합니다.
이 메서드는 코드베이스 전체에서 호출되지 않습니다. 또한 메서드 이름과 주석에서 "시스템 메시지 전용"이라고 명시하지만, type 파라미터는 JOIN/LEAVE/SYSTEM을 모두 받을 수 있어 목적이 불명확합니다. saveJoinLeave 메서드가 JOIN/LEAVE 유형을 처리하므로, saveSystem은 향후 순수 SYSTEM 타입 전용으로 예약된 것인지, 아니면 불필요한 코드인지 확인하여 제거하거나 명확히 하세요.
🤖 Prompt for AI Agents
In `@src/main/java/com/be/sportizebe/domain/chat/service/ChatMessageService.java`
around lines 67 - 79, The saveSystem method in ChatMessageService appears to be
dead/ambiguous: either remove it or make its purpose explicit; either (A) delete
the unused saveSystem(ChatRoom, ChatMessage.Type, String) and any related tests
if no callers exist, or (B) restrict and document it to only create pure system
messages by changing the signature to accept only ChatMessage.Type.SYSTEM
(remove JOIN/LEAVE from allowed values), add a clear javadoc comment, and ensure
callers use saveJoinLeave for JOIN/LEAVE while routing true system notifications
to this method; update any references to use messageRepository.save(msg)
consistently and add a unit test demonstrating the intended behavior.
| @Component | ||
| public class ChatSessionRegistry { | ||
|
|
||
| public record Presence(Long roomId, Long userId, String nickname) {} | ||
|
|
||
| private final ConcurrentMap<String, Presence> store = new ConcurrentHashMap<>(); | ||
|
|
||
| public void put(String sessionId, Long roomId, Long userId, String nickname) { | ||
| store.put(sessionId, new Presence(roomId, userId, nickname)); | ||
| } | ||
|
|
||
| public Presence remove(String sessionId) { | ||
| return store.remove(sessionId); | ||
| } | ||
|
|
||
| public Presence get(String sessionId) { | ||
| return store.get(sessionId); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
비정상 연결 해제 시 세션 정리 필요
사용자가 /chat.leave를 호출하지 않고 브라우저를 닫거나 네트워크가 끊어지면 세션이 레지스트리에 남아 메모리 누수가 발생할 수 있습니다. WebSocket 연결 해제 이벤트를 처리하여 자동으로 세션을 정리하는 것을 권장합니다.
🛠️ 제안하는 수정 - WebSocket 이벤트 리스너 추가
별도의 이벤트 리스너 클래스를 생성하거나 기존 설정에 추가:
`@Component`
`@RequiredArgsConstructor`
public class WebSocketEventListener {
private final ChatSessionRegistry registry;
private final ChatMessageService chatMessageService;
private final ChatRoomService chatRoomService;
private final SimpMessagingTemplate messagingTemplate;
`@EventListener`
public void handleSessionDisconnect(SessionDisconnectEvent event) {
String sessionId = event.getSessionId();
ChatSessionRegistry.Presence presence = registry.remove(sessionId);
if (presence != null) {
// 자동 퇴장 메시지 브로드캐스트
ChatRoom room = chatRoomService.getOrThrow(presence.roomId());
String content = presence.nickname() + " 님이 퇴장했습니다.";
ChatMessage msg = chatMessageService.saveJoinLeave(
room, ChatMessage.Type.LEAVE,
presence.userId(), presence.nickname(), content
);
messagingTemplate.convertAndSend(
"/topic/chat/rooms/" + presence.roomId(),
ChatMessageResponse.from(msg)
);
}
}
}🤖 Prompt for AI Agents
In
`@src/main/java/com/be/sportizebe/domain/chat/websocket/ChatSessionRegistry.java`
around lines 8 - 26, The registry currently never cleans up when sockets
disconnect; add a WebSocket disconnect event listener that calls
ChatSessionRegistry.remove(sessionId) to clear Presence entries and broadcast a
leave message: implement a component (e.g., WebSocketEventListener) with an
`@EventListener` method handleSessionDisconnect(SessionDisconnectEvent) that
retrieves the sessionId, removes the Presence from ChatSessionRegistry, fetches
the ChatRoom via ChatRoomService, creates/saves a LEAVE ChatMessage via
ChatMessageService (using presence.userId() and presence.nickname()), and sends
the ChatMessageResponse over SimpMessagingTemplate to
"/topic/chat/rooms/{roomId}" so abandoned sessions are removed and clients are
notified.
| // 3) 브로드캐스트 | ||
| messagingTemplate.convertAndSend("/topic/chat/rooms/" + req.getRoomId(), msg); |
There was a problem hiding this comment.
ChatMessage 엔티티 직접 브로드캐스트 문제
send() 메서드는 ChatMessageResponse.from(saved)를 사용하지만, join()과 leave()는 ChatMessage 엔티티를 직접 전송합니다. 이로 인해:
- JSON 응답 형식이 일관되지 않음
room필드가FetchType.LAZY이므로LazyInitializationException발생 가능- 의도하지 않은 내부 데이터 노출 위험
🐛 제안하는 수정
// 3) 브로드캐스트
- messagingTemplate.convertAndSend("/topic/chat/rooms/" + req.getRoomId(), msg);
+ messagingTemplate.convertAndSend("/topic/chat/rooms/" + req.getRoomId(), ChatMessageResponse.from(msg));🤖 Prompt for AI Agents
In
`@src/main/java/com/be/sportizebe/domain/chat/websocket/ChatStompController.java`
around lines 57 - 58, The join() and leave() handlers are broadcasting
ChatMessage entities directly (via messagingTemplate.convertAndSend) which leads
to inconsistent JSON vs send() (which uses ChatMessageResponse.from(saved)),
potential LazyInitializationException for room (FetchType.LAZY), and data
leakage; change join() and leave() to map the ChatMessage entity to the same DTO
used in send() by calling ChatMessageResponse.from(...) (or an equivalent
converter) and broadcast that DTO instead, ensuring any needed associations are
initialized inside the transaction/service layer before conversion so no lazy
fields are accessed during serialization.
| ); | ||
| messagingTemplate.convertAndSend("/topic/chat/rooms/" + req.getRoomId(), msg); |
There was a problem hiding this comment.
동일한 엔티티 직접 브로드캐스트 문제
leave() 메서드에서도 동일한 문제가 있습니다. ChatMessageResponse.from(msg)를 사용해야 합니다.
🐛 제안하는 수정
- messagingTemplate.convertAndSend("/topic/chat/rooms/" + req.getRoomId(), msg);
+ messagingTemplate.convertAndSend("/topic/chat/rooms/" + req.getRoomId(), ChatMessageResponse.from(msg));🤖 Prompt for AI Agents
In
`@src/main/java/com/be/sportizebe/domain/chat/websocket/ChatStompController.java`
around lines 74 - 75, The leave() handler in ChatStompController is broadcasting
the raw entity instead of the DTO, causing the same direct-entity broadcast
issue; change the messagingTemplate.convertAndSend call in leave() to send
ChatMessageResponse.from(msg) (same pattern used elsewhere) so you convert the
domain object to the response DTO before broadcasting, updating the call that
currently constructs the destination "/topic/chat/rooms/" + req.getRoomId() and
uses msg.
| function subscribeRoom(){ | ||
| if (!stompClient) return debug("[WARN] connect first"); | ||
| if (!stompClient.connected) return debug("[WARN] not connected yet"); | ||
|
|
||
| if (currentSub){ | ||
| currentSub.unsubscribe(); | ||
| currentSub = null; | ||
| setSub("not subscribed"); | ||
| debug("[UNSUBSCRIBED] previous"); | ||
| } | ||
|
|
||
| const rid = roomIdValue(); | ||
| const dest = `/topic/chat/rooms/${rid}`; | ||
| $("roomTitle").textContent = `room: ${rid}`; | ||
|
|
||
| currentSub = stompClient.subscribe(dest, (frame) => { | ||
| // 서버 JSON 그대로 받음 | ||
| try{ | ||
| const data = JSON.parse(frame.body); | ||
| appendMessage(data); | ||
| }catch(e){ | ||
| // 혹시 JSON 아니면 텍스트로 표시 | ||
| appendMessage({ | ||
| senderNickname: "system", | ||
| senderUserId: -1, | ||
| content: frame.body | ||
| }); | ||
| } | ||
| }); | ||
| stompClient.send("/app/chat.join", {}, JSON.stringify({ | ||
| roomId: Number(roomIdValue()), | ||
| senderUserId: myUserId(), | ||
| senderNickname: myNickname() | ||
| })); | ||
| setSub("subscribed: " + dest); | ||
| debug("[SUBSCRIBED] " + dest); |
There was a problem hiding this comment.
roomId/userId 유효성 검증 추가 필요
roomId가 비어있거나 userId가 NaN이면 /topic/chat/rooms/로 구독하거나 join 페이로드가 비정상 값이 됩니다. 간단한 검증 후 반환하는 게 안전합니다.
🐛 제안 변경
- const rid = roomIdValue();
- const dest = `/topic/chat/rooms/${rid}`;
+ const rid = roomIdValue();
+ const roomIdNum = Number(rid);
+ const userIdNum = myUserId();
+ const nickname = myNickname();
+ if (!rid || !Number.isFinite(roomIdNum) || !Number.isFinite(userIdNum) || !nickname) {
+ return debug("[WARN] invalid roomId/userId/nickname");
+ }
+ const dest = `/topic/chat/rooms/${roomIdNum}`;
- stompClient.send("/app/chat.join", {}, JSON.stringify({
- roomId: Number(roomIdValue()),
- senderUserId: myUserId(),
- senderNickname: myNickname()
- }));
+ stompClient.send("/app/chat.join", {}, JSON.stringify({
+ roomId: roomIdNum,
+ senderUserId: userIdNum,
+ senderNickname: nickname
+ }));📝 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.
| function subscribeRoom(){ | |
| if (!stompClient) return debug("[WARN] connect first"); | |
| if (!stompClient.connected) return debug("[WARN] not connected yet"); | |
| if (currentSub){ | |
| currentSub.unsubscribe(); | |
| currentSub = null; | |
| setSub("not subscribed"); | |
| debug("[UNSUBSCRIBED] previous"); | |
| } | |
| const rid = roomIdValue(); | |
| const dest = `/topic/chat/rooms/${rid}`; | |
| $("roomTitle").textContent = `room: ${rid}`; | |
| currentSub = stompClient.subscribe(dest, (frame) => { | |
| // 서버 JSON 그대로 받음 | |
| try{ | |
| const data = JSON.parse(frame.body); | |
| appendMessage(data); | |
| }catch(e){ | |
| // 혹시 JSON 아니면 텍스트로 표시 | |
| appendMessage({ | |
| senderNickname: "system", | |
| senderUserId: -1, | |
| content: frame.body | |
| }); | |
| } | |
| }); | |
| stompClient.send("/app/chat.join", {}, JSON.stringify({ | |
| roomId: Number(roomIdValue()), | |
| senderUserId: myUserId(), | |
| senderNickname: myNickname() | |
| })); | |
| setSub("subscribed: " + dest); | |
| debug("[SUBSCRIBED] " + dest); | |
| function subscribeRoom(){ | |
| if (!stompClient) return debug("[WARN] connect first"); | |
| if (!stompClient.connected) return debug("[WARN] not connected yet"); | |
| if (currentSub){ | |
| currentSub.unsubscribe(); | |
| currentSub = null; | |
| setSub("not subscribed"); | |
| debug("[UNSUBSCRIBED] previous"); | |
| } | |
| const rid = roomIdValue(); | |
| const roomIdNum = Number(rid); | |
| const userIdNum = myUserId(); | |
| const nickname = myNickname(); | |
| if (!rid || !Number.isFinite(roomIdNum) || !Number.isFinite(userIdNum) || !nickname) { | |
| return debug("[WARN] invalid roomId/userId/nickname"); | |
| } | |
| const dest = `/topic/chat/rooms/${roomIdNum}`; | |
| $("roomTitle").textContent = `room: ${rid}`; | |
| currentSub = stompClient.subscribe(dest, (frame) => { | |
| // 서버 JSON 그대로 받음 | |
| try{ | |
| const data = JSON.parse(frame.body); | |
| appendMessage(data); | |
| }catch(e){ | |
| // 혹시 JSON 아니면 텍스트로 표시 | |
| appendMessage({ | |
| senderNickname: "system", | |
| senderUserId: -1, | |
| content: frame.body | |
| }); | |
| } | |
| }); | |
| stompClient.send("/app/chat.join", {}, JSON.stringify({ | |
| roomId: roomIdNum, | |
| senderUserId: userIdNum, | |
| senderNickname: nickname | |
| })); | |
| setSub("subscribed: " + dest); | |
| debug("[SUBSCRIBED] " + dest); |
🤖 Prompt for AI Agents
In `@src/main/java/com/be/sportizebe/test/stomp-test.html` around lines 433 - 468,
In subscribeRoom(), validate the roomId and userId before building the
destination or sending join: call roomIdValue() and ensure it's non-empty and
Number(roomIdValue()) is not NaN, and call myUserId() and ensure it's a finite
number (not NaN); if either check fails, log a debug/warn and return early
instead of subscribing or sending the /app/chat.join payload. Also ensure
destination string construction uses the validated numeric roomId and include
these checks near the top of subscribeRoom (before dest/currentSub logic)
referencing subscribeRoom, roomIdValue(), myUserId(), stompClient, currentSub
and stompClient.send.
|
join, leave 되어 있는데요? |
#️⃣ Issue Number
📝 요약(Summary)
🛠️ PR 유형
어떤 변경 사항이 있나요?
📸스크린샷 (선택)
💬 공유사항 to 리뷰어
✅ PR Checklist
PR이 다음 요구 사항을 충족하는지 확인하세요.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.