Skip to content

Conversation

@mete0rfish
Copy link
Member

@mete0rfish mete0rfish commented Dec 8, 2025

PR Type

Enhancement


Description

  • Implemented STOMP for real-time chat.

  • Replaced WebSocketHandler with STOMP messaging.

  • Modified ChatRoom to use Redis Hash for storage.

  • Added RedisSubscriber for message handling.


Diagram Walkthrough

flowchart LR
  A["ChatController (STOMP)"] -- "sendMessage" --> B["ChatService"]
  B -- "saveMessage, publishMessage" --> C["Redis"]
  C -- "RedisSubscriber" --> D["SimpMessageSendingOperations"]
  D -- "convertAndSend" --> E["/sub/chat/room/{roomId}"]
Loading

File Walkthrough

Relevant files
Enhancement
4 files
ChatController.java
Migrated to STOMP messaging in `ChatController`                   
+10/-1   
ChatRoom.java
Modified `ChatRoom` to use Redis Hash                                       
+9/-11   
ChatService.java
Updated `ChatService` for STOMP and Redis Hash                     
+65/-63 
RedisSubscriber.java
Added `RedisSubscriber` for message handling                         
+32/-0   
Miscellaneous
3 files
ChatWebSocketHandler.java
Removed `ChatWebSocketHandler`                                                     
+0/-134 
RedisPublisher.java
Removed `RedisPublisher`                                                                 
+0/-32   
RedisSubscriber.java
Removed `RedisSubscriber`                                                               
+0/-53   
Configuration changes
3 files
SecurityConfig.java
Added `/ws-stomp/**` to authentication whitelist                 
+1/-1     
WebSocketConfig.java
Configured WebSocket for STOMP messaging                                 
+12/-20 
RedisConfig.java
Configured Redis for chat and removed unused code               
+11/-13 

@mete0rfish
Copy link
Member Author

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Double Serialization

The code appears to be serializing the ChatRoom object twice when storing it in Redis. This could lead to issues when retrieving and deserializing the object. Consider removing the outer serialization to avoid this.

return opsHashChatRoom.values(CHAT_ROOMS).stream().map(room -> {
    try {
        // 이중으로 직렬화된 JSON을 처리하기 위해, 먼저 내부 JSON 문자열을 추출
        String innerJson = objectMapper.readValue(room, String.class);
        // 추출된 JSON 문자열을 실제 ChatRoom 객체로 변환
        return objectMapper.readValue(innerJson, ChatRoom.class);
    } catch (JsonProcessingException e) {
        log.error("채팅방 목록 조회에 실패했습니다.", e);
        return null;
    }
}).filter(Objects::nonNull).collect(Collectors.toList());
Error Handling

The findAllRoom and findRoomById methods include error handling for JsonProcessingException, but they return null in case of failure. Returning null could lead to NullPointerExceptions in other parts of the application. Consider throwing a custom exception or returning an empty list instead.

public List<ChatRoom> findAllRoom() {
    return opsHashChatRoom.values(CHAT_ROOMS).stream().map(room -> {
        try {
            // 이중으로 직렬화된 JSON을 처리하기 위해, 먼저 내부 JSON 문자열을 추출
            String innerJson = objectMapper.readValue(room, String.class);
            // 추출된 JSON 문자열을 실제 ChatRoom 객체로 변환
            return objectMapper.readValue(innerJson, ChatRoom.class);
        } catch (JsonProcessingException e) {
            log.error("채팅방 목록 조회에 실패했습니다.", e);
            return null;
        }
    }).filter(Objects::nonNull).collect(Collectors.toList());
}

public ChatRoom findRoomById(String roomId) {
    try {
        String roomJson = opsHashChatRoom.get(CHAT_ROOMS, roomId);
        // findRoomById도 이중 직렬화 문제를 겪을 수 있으므로 동일하게 수정
        String innerJson = objectMapper.readValue(roomJson, String.class);
        return objectMapper.readValue(innerJson, ChatRoom.class);
    } catch (JsonProcessingException | IllegalArgumentException e) {
        log.error("ID로 채팅방을 찾는데 실패했습니다.", e);
        return null;
    }
}

@mete0rfish
Copy link
Member Author

Failed to generate code suggestions for PR

@mete0rfish mete0rfish merged commit 2087166 into main Dec 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants