Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions assets/stomp-test.html
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,8 @@
});
stompClient.send("/app/chat.join", {}, JSON.stringify({
roomId: Number(roomIdValue()),
senderUserId: myUserId(),
senderNickname: myNickname()
userId: myUserId(),
nickname: myNickname()
}));
setSub("subscribed: " + dest);
debug("[SUBSCRIBED] " + dest);
Expand Down Expand Up @@ -503,8 +503,8 @@
if (stompClient && stompClient.connected) {
stompClient.send("/app/chat.leave", {}, JSON.stringify({
roomId: Number(roomIdValue()),
senderUserId: myUserId(),
senderNickname: myNickname()
userId: myUserId(),
nickname: myNickname()
}));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.be.sportizebe.domain.chat.dto;

import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor
public class ChatPresenceRequest {
private Long roomId;
private Long userId;
private String nickname;
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,23 @@ public static ChatMessage system(ChatRoom room, String content) {
.content(content)
.build();
}
public static ChatMessage join(ChatRoom room, Long senderUserId, String senderNickname) {
return ChatMessage.builder()
.room(room)
.senderUserId(senderUserId)
.senderNickname(senderNickname)
.type(Type.JOIN)
.content(null)
.build();
}

public static ChatMessage leave(ChatRoom room, Long senderUserId, String senderNickname) {
return ChatMessage.builder()
.room(room)
.senderUserId(senderUserId)
.senderNickname(senderNickname)
.type(Type.LEAVE)
.content(null)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,28 @@ public CursorPageResponse<ChatMessageResponse> list(Long roomId, Long beforeId,
.build();

}
// 채팅이 아닌 시스템 메시지 전용!
@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);
}
Comment on lines +67 to +79
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for saveSystem method usage in Java files
rg -n "saveSystem" --type=java -B 2 -A 2

Repository: 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.


@Transactional
public ChatMessage saveJoinLeave(ChatRoom room, ChatMessage.Type type, Long userId, String nickname, String content) {
return messageRepository.save(ChatMessage.builder()
.room(room)
.senderUserId(userId)
.senderNickname(nickname)
.type(type) // JOIN / LEAVE
.content(content)
.build());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.be.sportizebe.domain.chat.websocket;

import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import org.springframework.stereotype.Component;

@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);
}
}
Comment on lines +8 to +26
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

비정상 연결 해제 시 세션 정리 필요

사용자가 /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.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package com.be.sportizebe.domain.chat.websocket;

import com.be.sportizebe.domain.chat.dto.ChatPresenceRequest;
import com.be.sportizebe.domain.chat.dto.response.ChatMessageResponse;
import com.be.sportizebe.domain.chat.dto.request.ChatSendRequest;
import com.be.sportizebe.domain.chat.entity.ChatMessage;
Expand All @@ -8,6 +8,7 @@
import com.be.sportizebe.domain.chat.service.ChatRoomService;
import lombok.RequiredArgsConstructor;
import org.springframework.messaging.handler.annotation.MessageMapping;
import org.springframework.messaging.simp.SimpMessageHeaderAccessor;
import org.springframework.messaging.simp.SimpMessagingTemplate;
import org.springframework.stereotype.Controller;

Expand All @@ -17,6 +18,7 @@ public class ChatStompController {
private final ChatMessageService chatMessageService;
private final ChatRoomService chatRoomService; // ✅ 추가
private final SimpMessagingTemplate messagingTemplate;
private final ChatSessionRegistry registry;

@MessageMapping("/chat.send")
public void send(ChatSendRequest req){
Expand All @@ -33,4 +35,43 @@ public void send(ChatSendRequest req){
ChatMessageResponse.from(saved)
);
}

@MessageMapping("/chat.join")
public void join(ChatPresenceRequest req, SimpMessageHeaderAccessor headerAccessor) {
String sessionId = headerAccessor.getSessionId();

ChatRoom room = chatRoomService.getOrThrow(req.getRoomId());

// 1) 세션에 “이 탭이 어느 방에 있는지” 저장
registry.put(sessionId, req.getRoomId(), req.getUserId(), req.getNickname());

// 2) 시스템 메시지 생성 + (옵션) DB 저장
String content = req.getNickname() + " 님이 입장했습니다.";
ChatMessage msg = chatMessageService.saveJoinLeave(
room,
ChatMessage.Type.JOIN,
req.getUserId(),
req.getNickname(),
content
);
// 3) 브로드캐스트
messagingTemplate.convertAndSend("/topic/chat/rooms/" + req.getRoomId(), msg);
Comment on lines +57 to +58
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

ChatMessage 엔티티 직접 브로드캐스트 문제

send() 메서드는 ChatMessageResponse.from(saved)를 사용하지만, join()leave()ChatMessage 엔티티를 직접 전송합니다. 이로 인해:

  1. JSON 응답 형식이 일관되지 않음
  2. room 필드가 FetchType.LAZY이므로 LazyInitializationException 발생 가능
  3. 의도하지 않은 내부 데이터 노출 위험
🐛 제안하는 수정
         // 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.

}
@MessageMapping("/chat.leave")
public void leave(ChatPresenceRequest req, SimpMessageHeaderAccessor headerAccessor) {
String sessionId = headerAccessor.getSessionId();

ChatRoom room = chatRoomService.getOrThrow(req.getRoomId());
registry.remove(sessionId); // 명시적으로 나가면 제거

String content = req.getNickname() + " 님이 퇴장했습니다.";
ChatMessage msg = chatMessageService.saveJoinLeave(
room,
ChatMessage.Type.LEAVE,
req.getUserId(),
req.getNickname(),
content
);
messagingTemplate.convertAndSend("/topic/chat/rooms/" + req.getRoomId(), msg);
Comment on lines +74 to +75
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

동일한 엔티티 직접 브로드캐스트 문제

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.

}
}
Loading