Conversation
Walkthrough이번 변경 사항은 1:1 채팅 기능의 도입을 위한 대규모 신규 코드 추가입니다. WebSocket 및 STOMP 기반 실시간 메시징, 채팅방 및 메시지 엔티티, 관련 레포지토리, 서비스, REST 및 WebSocket 컨트롤러, DTO, 인증 인터셉터, 에러 코드 등이 새로 구현되었습니다. 기존 코드 변경은 없으며, 모든 기능은 신규 파일로 구성되어 있습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WebSocketEndpoint
participant WebSocketAuthInterceptor
participant StompChannelInterceptor
participant ChatService
participant MessageBroker
Client->>WebSocketEndpoint: STOMP CONNECT (/ws/chat)
WebSocketEndpoint->>WebSocketAuthInterceptor: beforeHandshake (JWT 인증)
WebSocketAuthInterceptor-->>WebSocketEndpoint: 인증 성공/실패
WebSocketEndpoint->>StompChannelInterceptor: preSend (CONNECT)
StompChannelInterceptor-->>WebSocketEndpoint: Principal 설정
Client->>WebSocketEndpoint: SEND /app/chat/{roomId}/sendMessage
WebSocketEndpoint->>StompChannelInterceptor: preSend (메시지)
StompChannelInterceptor-->>WebSocketEndpoint: 인증/권한 체크
WebSocketEndpoint->>ChatService: sendMessage()
ChatService-->>WebSocketEndpoint: ChatMessageDto 반환
WebSocketEndpoint->>MessageBroker: /topic/chat/{roomId}로 메시지 브로드캐스트
MessageBroker-->>Client: SUBSCRIBE된 모든 클라이언트에 메시지 전달
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @yooooonshine, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a complete 1:1 real-time chat system into the application. It establishes the necessary infrastructure for WebSocket communication, integrates robust JWT-based security for chat interactions, and provides a full suite of features for managing chat rooms and messages, including persistence, pagination, and soft deletion. The changes are designed to support a single-server environment and enhance user interaction through instant messaging capabilities.
Highlights
- Real-time Chat System Implementation: Implemented a 1:1 real-time chat system leveraging WebSocket and STOMP protocols, enabling instant message exchange between users.
- Secure Communication with JWT: Integrated JWT-based authentication for WebSocket connections and STOMP message transmission, ensuring secure and authorized access to chat functionalities.
- Comprehensive Chat Room Management: Introduced functionalities for creating new chat rooms (or retrieving existing ones), listing active chat rooms for a user, and a soft-delete mechanism for chat rooms, allowing individual users to 'leave' a conversation without permanently deleting it for the other participant.
- Advanced Message Handling: Developed features for sending messages, retrieving messages with pagination, marking messages as read, and tracking unread message counts within chat rooms.
- Dedicated Data Models and Persistence: Designed and implemented new JPA entities (
ChatRoomEntity,ChatMessageEntity) and their corresponding repositories (ChatRoomRepository,ChatMessageRepository) to efficiently store and manage chat data, optimized withLongtype IDs. - RESTful and WebSocket API Endpoints: Exposed both REST API endpoints for chat room and message management (e.g.,
/api/chat/rooms,/api/chat/messages) and WebSocket API endpoints for real-time message sending and user presence (/app/chat.sendMessage,/app/chat.addUser). - Enhanced Error Handling: Added specific error codes (
CH-001toCH-004) toErrorCode.javato provide clear and precise feedback for chat-related exceptions, such as chat room not found or access denied.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The code changes introduce a 1:1 real-time chat system using WebSocket + STOMP. The implementation includes JWT authentication, a data model, REST APIs, and WebSocket APIs. Several improvements are suggested, including enhancing security by restricting allowed origins, refining exception handling, optimizing database queries to avoid N+1 problems, and improving error feedback to clients.
| try { | ||
| Long senderId = Long.parseLong(principal.getName()); | ||
|
|
||
| ChatMessageDto chatMessage = chatService.sendMessage(roomId, senderId, message.getMessage()); | ||
|
|
||
| messagingTemplate.convertAndSend("/topic/chat/" + roomId, chatMessage); | ||
|
|
||
| } catch (Exception e) { | ||
| log.error("메시지 전송 중 오류 발생: {}", e.getMessage()); | ||
| } |
There was a problem hiding this comment.
The try-catch block here catches all exceptions and simply logs them. This means that if an error occurs during message processing (e.g., database error, validation failure), the message is silently dropped, and the sending client receives no feedback. This is a poor user experience and makes debugging difficult.
You should implement a proper error handling strategy for STOMP. A common pattern is to send an error message back to the user on a private, user-specific destination (e.g., /user/queue/errors). This ensures the client is aware of the failure.
| @NoArgsConstructor | ||
| public class MarkMessagesAsReadRequest { | ||
|
|
||
| @NotBlank(message = "채팅방 ID는 필수입니다.") |
There was a problem hiding this comment.
The @NotBlank annotation is intended for String fields. Using it on a Long field will cause a jakarta.validation.UnexpectedTypeException at runtime. For non-string types where you just want to check for null, you should use @NotNull.
| @NotBlank(message = "채팅방 ID는 필수입니다.") | |
| @NotNull(message = "채팅방 ID는 필수입니다.") |
| @Override | ||
| public void registerStompEndpoints(StompEndpointRegistry registry) { | ||
| registry.addEndpoint("/ws/chat") | ||
| .setAllowedOriginPatterns("*") // TODO: 실제 도메인으로 변경 필요 |
There was a problem hiding this comment.
Using "*" for allowed origins is a significant security risk as it allows any website to connect to your WebSocket endpoint. This can lead to Cross-Site WebSocket Hijacking (CSWH) attacks. For production environments, you should restrict this to a specific list of allowed domains for your frontend application.
| return chatRooms.stream() | ||
| .map(room -> { | ||
| final UserEntity opponent = getUserById(room.getOpponent(userId)); | ||
| final ChatMessageEntity lastMessage = chatMessageRepository.findLatestMessageByChatRoomId(room.getId()) | ||
| .orElse(null); | ||
| final Long unreadCount = chatMessageRepository.countUnreadMessages(room.getId(), userId); | ||
|
|
||
| return GetChatRoomsResponse.builder() | ||
| .chatRoomId(room.getId()) | ||
| .opponent(GetChatRoomsResponse.OpponentDto.builder() | ||
| .userId(opponent.getId()) | ||
| .nickname(opponent.getName()) | ||
| .profileImageUrl(opponent.getUserImageUrl()) | ||
| .build()) | ||
| .lastMessage(lastMessage != null ? lastMessage.getMessage() : null) | ||
| .lastMessageTime(lastMessage != null ? lastMessage.getCreateAt() : room.getCreateAt()) | ||
| .unreadCount(unreadCount) | ||
| .build(); | ||
| }) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
This method has a potential N+1 query problem. For each chat room, it executes two additional queries inside the loop: findLatestMessageByChatRoomId and countUnreadMessages. If a user has N chat rooms, this will result in 1 + 2*N queries, which can lead to significant performance degradation as the number of chat rooms grows.
To optimize this, you could fetch all the necessary data in a smaller number of queries before the loop. For instance, you could fetch all latest messages and unread counts for the user's active rooms at once and then map them to the corresponding chat rooms in memory.
| } catch (Exception e) { | ||
| log.warn("StompChannelInterceptor - STOMP 연결 인증 실패: {}", e.getMessage()); | ||
| return null; |
There was a problem hiding this comment.
Catching the generic Exception is too broad. It can mask underlying issues and make debugging more difficult because it catches both checked and unchecked exceptions. It's better to catch more specific exceptions that you expect from the token validation logic, such as AppException or specific exceptions from the JWT library. This will lead to more robust error handling and clearer logs.
| } catch (Exception e) { | ||
| log.error("WebSocket 토큰 파싱 실패: {}", e.getMessage()); | ||
| } |
There was a problem hiding this comment.
Similar to the StompChannelInterceptor, catching a generic Exception is too broad. This can hide unexpected runtime exceptions and makes it harder to distinguish between different types of errors (e.g., an expired token vs. a malformed token). Please catch more specific exceptions to improve error handling and logging.
| @Query("SELECT cm FROM ChatMessageEntity cm " + | ||
| "WHERE cm.chatRoomId = :chatRoomId " + | ||
| "ORDER BY cm.createAt DESC " + | ||
| "LIMIT 1") |
There was a problem hiding this comment.
The LIMIT 1 clause is not part of the standard JPQL specification. While it may be supported by your current JPA provider (Hibernate) for certain database dialects, it reduces the portability of your code to other databases.
A more standard and portable approach would be to use a Spring Data JPA derived query method, which is also more readable.
| final List<ChatMessageEntity> unreadMessages = chatMessageRepository.findUnreadMessages(chatRoomId, userId); | ||
|
|
||
| for (final ChatMessageEntity message : unreadMessages) { | ||
| if (message.getId() <= lastReadMessage.getId()) { | ||
| message.markAsRead(); | ||
| } | ||
| } | ||
|
|
||
| chatMessageRepository.saveAll(unreadMessages); |
There was a problem hiding this comment.
This implementation fetches a list of unread messages into memory, iterates through them, and then saves them back to the database. This can be inefficient if there are many unread messages.
A more performant approach is to issue a single UPDATE statement to mark all relevant messages as read directly in the database.
| @MessageMapping("/chat.addUser/{roomId}") | ||
| public void addUser( | ||
| @DestinationVariable Long roomId, | ||
| Principal principal | ||
| ) { | ||
| try { | ||
| Long userId = Long.parseLong(principal.getName()); | ||
|
|
||
| // 채팅방 참여 권한 검증 | ||
| chatService.validateUserAccessToChatRoom(roomId, userId); | ||
|
|
||
| log.info("사용자 {}가 채팅방 {}에 입장했습니다.", userId, roomId); | ||
|
|
||
| } catch (Exception e) { | ||
| log.error("사용자 입장 처리 중 오류 발생: {}", e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
The addUser method currently only validates access and logs an info message. The ChatMessageDto.MessageType enum contains JOIN and LEAVE types, which suggests that the system is intended to broadcast user presence events.
Consider enhancing this method to create and send a ChatMessageDto with type JOIN to the chat room's topic. This would allow all clients in the room to be notified when a new user joins, improving the user experience.
| @NotNull(message = "수신자 ID는 필수입니다.") | ||
| private Long receiverId; |
There was a problem hiding this comment.
The receiverId field in this request DTO is not used in the ChatWebSocketController.sendMessage method. The controller identifies the chat room using the roomId from the destination path. This field is redundant and could be confusing for API consumers. It's best to remove it to make the DTO's contract clearer.
![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 (13)
src/main/java/hanium/modic/backend/web/chat/dto/request/SendMessageRequest.java (1)
1-17: DTO 구조가 잘 설계되었습니다.
SendMessageRequestDTO가 적절히 구현되었습니다. 유효성 검사 어노테이션이 올바르게 적용되어 있고, Lombok을 활용한 보일러플레이트 코드 감소도 좋습니다.다만 유효성 검사 메시지가 한국어로 하드코딩되어 있는데, 국제화(i18n)를 고려해보시기 바랍니다.
국제화를 위한 메시지 키 사용 예시:
- @NotBlank(message = "메시지 내용은 필수입니다.") + @NotBlank(message = "{validation.message.required}") private String message; - @NotNull(message = "수신자 ID는 필수입니다.") + @NotNull(message = "{validation.receiverId.required}") private Long receiverId;src/main/java/hanium/modic/backend/web/chat/controller/ChatWebSocketController.java (2)
43-59: 사용자 입장 처리 메서드 개선을 고려해보세요.접근 권한 검증과 로깅은 잘 구현되어 있지만, 다른 사용자들에게 입장 알림을 보내는 것을 고려해볼 수 있습니다.
사용자 입장 시 알림 메시지를 보내려면 다음과 같이 추가할 수 있습니다:
// 채팅방 참여 권한 검증 chatService.validateUserAccessToChatRoom(roomId, userId); + // 입장 알림 메시지 전송 (선택적) + ChatMessageDto joinMessage = ChatMessageDto.builder() + .roomId(roomId.toString()) + .senderId(userId) + .type(ChatMessageDto.MessageType.JOIN) + .timestamp(LocalDateTime.now()) + .build(); + messagingTemplate.convertAndSend("/topic/chat/" + roomId, joinMessage); + log.info("사용자 {}가 채팅방 {}에 입장했습니다.", userId, roomId);
61-77: 사용자 퇴장 처리 메서드도 알림 기능 추가를 고려해보세요.addUser 메서드와 마찬가지로 다른 사용자들에게 퇴장 알림을 보내는 것을 고려해볼 수 있습니다.
사용자 퇴장 시 알림 메시지를 보내려면 다음과 같이 추가할 수 있습니다:
// 채팅방 참여 권한 검증 chatService.validateUserAccessToChatRoom(roomId, userId); + // 퇴장 알림 메시지 전송 (선택적) + ChatMessageDto leaveMessage = ChatMessageDto.builder() + .roomId(roomId.toString()) + .senderId(userId) + .type(ChatMessageDto.MessageType.LEAVE) + .timestamp(LocalDateTime.now()) + .build(); + messagingTemplate.convertAndSend("/topic/chat/" + roomId, leaveMessage); + log.info("사용자 {}가 채팅방 {}에서 나갔습니다.", userId, roomId);src/main/java/hanium/modic/backend/domain/chat/entity/ChatMessageEntity.java (2)
47-54: Builder 패턴 개선 제안Builder가 엔티티를 받아서 ID만 추출하는 방식은 불필요한 엔티티 조회를 유발할 수 있습니다. ID를 직접 받는 오버로드 메서드를 추가하는 것을 고려해보세요.
@Builder(builderMethodName = "entityBuilder") private ChatMessageEntity(String messageId, ChatRoomEntity chatRoom, UserEntity sender, String message) { this.messageId = messageId; this.chatRoomId = chatRoom.getId(); this.senderId = sender.getId(); this.message = message; this.isRead = false; } @Builder private ChatMessageEntity(String messageId, Long chatRoomId, Long senderId, String message) { this.messageId = messageId; this.chatRoomId = chatRoomId; this.senderId = senderId; this.message = message; this.isRead = false; }
40-42: 대용량 메시지 처리 고려사항
@Lob사용 시 대용량 메시지로 인한 성능 문제가 발생할 수 있습니다. 메시지 크기 제한이나 별도 테이블 분리를 고려해보세요.src/main/java/hanium/modic/backend/common/websocket/interceptor/StompChannelInterceptor.java (1)
57-58: Principal 생성 시 권한 정보 추가 고려현재
UsernamePasswordAuthenticationToken에 권한(authorities) 정보가 없습니다. 향후 권한 기반 접근 제어가 필요할 경우를 대비해 사용자 권한을 포함시키는 것을 고려해보세요.src/main/java/hanium/modic/backend/common/websocket/config/WebSocketConfig.java (1)
43-46: Bean 생성 방식 개선 제안
WebSocketAuthInterceptor를 수동으로 생성하는 대신@Component로 등록하여 자동 주입받는 것이 Spring의 DI 원칙에 더 부합합니다.
WebSocketAuthInterceptor클래스에@Component어노테이션을 추가하고, 이 메서드를 제거하세요.src/main/java/hanium/modic/backend/domain/chat/entity/ChatRoomEntity.java (1)
27-31: 데이터베이스 인덱스 추가 권장채팅방 조회 성능 향상을 위해 user1_id와 user2_id에 복합 인덱스를 추가하는 것을 권장합니다.
@Table(name = "chat_rooms", indexes = { @Index(name = "idx_user1_user2", columnList = "user1_id, user2_id"), @Index(name = "idx_user2", columnList = "user2_id") })src/main/java/hanium/modic/backend/common/websocket/interceptor/WebSocketAuthInterceptor.java (2)
72-81: 쿼리 파라미터 파싱 개선URL 쿼리 파라미터 파싱을 더 안전하고 효율적으로 개선할 수 있습니다.
-// 실패 시 쿼리 파라미터에서 token 추출 -String queryToken = request.getURI().getQuery(); -if (queryToken != null && queryToken.contains("token=")) { - String[] params = queryToken.split("&"); - for (String param : params) { - if (param.startsWith("token=")) { - return param.substring(6); - } - } -} +// 실패 시 쿼리 파라미터에서 token 추출 +String query = request.getURI().getQuery(); +if (query != null) { + return Arrays.stream(query.split("&")) + .filter(param -> param.startsWith("token=")) + .map(param -> param.substring(6)) + .findFirst() + .orElse(null); +}
54-62: 빈 메서드 구현체 제거 제안
afterHandshake메서드가 비어있다면, 인터페이스의 default 메서드를 활용하거나 명시적으로 비어있음을 표시하는 것이 좋습니다.@Override public void afterHandshake( ServerHttpRequest request, ServerHttpResponse response, WebSocketHandler wsHandler, Exception exception ) { - // 핸드셰이크 후 처리 로직 + // 현재 구현 필요 없음 }src/main/java/hanium/modic/backend/domain/chat/service/ChatService.java (2)
159-166: 메시지 읽음 처리 성능 개선 가능현재 구현은 각 메시지를 개별적으로 확인하고 있습니다. 대량의 읽지 않은 메시지가 있을 경우 성능 이슈가 발생할 수 있습니다.
일괄 업데이트 쿼리를 사용하여 성능을 개선할 수 있습니다:
// ChatMessageRepository에 추가 @Modifying @Query("UPDATE ChatMessageEntity cm SET cm.isRead = true " + "WHERE cm.chatRoomId = :chatRoomId " + "AND cm.senderId != :userId " + "AND cm.isRead = false " + "AND cm.id <= :lastReadMessageId") void markMessagesAsReadBatch(@Param("chatRoomId") Long chatRoomId, @Param("userId") Long userId, @Param("lastReadMessageId") Long lastReadMessageId);그리고 서비스 메서드를 다음과 같이 수정:
-final List<ChatMessageEntity> unreadMessages = chatMessageRepository.findUnreadMessages(chatRoomId, userId); - -for (final ChatMessageEntity message : unreadMessages) { - if (message.getId() <= lastReadMessage.getId()) { - message.markAsRead(); - } -} - -chatMessageRepository.saveAll(unreadMessages); +chatMessageRepository.markMessagesAsReadBatch(chatRoomId, userId, lastReadMessage.getId());
196-201: 메서드명이 실제 동작과 불일치
validateDeleteChatRoom메서드명은 "채팅방 삭제를 검증한다"로 오해될 수 있습니다. 실제로는 채팅방이 이미 삭제되었는지 확인하는 메서드입니다.더 명확한 메서드명으로 변경하는 것을 제안합니다:
-private void validateDeleteChatRoom(final ChatRoomEntity chatRoom, final Long userId) { +private void validateChatRoomNotDeleted(final ChatRoomEntity chatRoom, final Long userId) {그리고 이 메서드를 호출하는 곳들도 함께 수정:
-validateDeleteChatRoom(chatRoom, userId); +validateChatRoomNotDeleted(chatRoom, userId);src/main/java/hanium/modic/backend/web/chat/controller/ChatController.java (1)
119-119: REST API 경로 일관성 개선 필요현재 채팅방 관련 엔드포인트들이
/rooms를 사용하는데, 삭제 엔드포인트만/room을 사용하고 있습니다. API 일관성을 위해 복수형으로 통일하는 것이 좋습니다.-@DeleteMapping("/room/{chatRoomId}") +@DeleteMapping("/rooms/{chatRoomId}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
build.gradle(1 hunks)src/main/java/hanium/modic/backend/common/error/ErrorCode.java(1 hunks)src/main/java/hanium/modic/backend/common/websocket/config/WebSocketConfig.java(1 hunks)src/main/java/hanium/modic/backend/common/websocket/interceptor/StompChannelInterceptor.java(1 hunks)src/main/java/hanium/modic/backend/common/websocket/interceptor/WebSocketAuthInterceptor.java(1 hunks)src/main/java/hanium/modic/backend/domain/chat/entity/ChatMessageEntity.java(1 hunks)src/main/java/hanium/modic/backend/domain/chat/entity/ChatRoomEntity.java(1 hunks)src/main/java/hanium/modic/backend/domain/chat/repository/ChatMessageRepository.java(1 hunks)src/main/java/hanium/modic/backend/domain/chat/repository/ChatRoomRepository.java(1 hunks)src/main/java/hanium/modic/backend/domain/chat/service/ChatService.java(1 hunks)src/main/java/hanium/modic/backend/web/chat/controller/ChatController.java(1 hunks)src/main/java/hanium/modic/backend/web/chat/controller/ChatWebSocketController.java(1 hunks)src/main/java/hanium/modic/backend/web/chat/dto/dto/ChatMessageDto.java(1 hunks)src/main/java/hanium/modic/backend/web/chat/dto/request/MarkMessagesAsReadRequest.java(1 hunks)src/main/java/hanium/modic/backend/web/chat/dto/request/SendMessageRequest.java(1 hunks)src/main/java/hanium/modic/backend/web/chat/dto/response/ChatRoomCreateResponse.java(1 hunks)src/main/java/hanium/modic/backend/web/chat/dto/response/GetChatRoomsResponse.java(1 hunks)src/main/java/hanium/modic/backend/web/chat/dto/response/GetMessagesResponse.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/hanium/modic/backend/web/chat/dto/response/GetChatRoomsResponse.java (4)
src/main/java/hanium/modic/backend/web/chat/dto/request/MarkMessagesAsReadRequest.java (1)
Getter(7-16)src/main/java/hanium/modic/backend/web/chat/dto/dto/ChatMessageDto.java (1)
Getter(8-25)src/main/java/hanium/modic/backend/web/chat/dto/response/ChatRoomCreateResponse.java (1)
Getter(6-11)src/main/java/hanium/modic/backend/web/chat/dto/request/SendMessageRequest.java (1)
Getter(8-17)
src/main/java/hanium/modic/backend/common/websocket/interceptor/StompChannelInterceptor.java (2)
src/main/java/hanium/modic/backend/common/websocket/interceptor/WebSocketAuthInterceptor.java (1)
Slf4j(20-85)src/main/java/hanium/modic/backend/web/chat/controller/ChatWebSocketController.java (1)
Slf4j(17-78)
🔇 Additional comments (12)
build.gradle (1)
64-65: WebSocket 의존성 추가가 적절합니다.채팅 시스템 구현을 위한 Spring Boot WebSocket starter 의존성이 올바르게 추가되었습니다. 주석과 함께 적절한 위치에 배치되어 있습니다.
src/main/java/hanium/modic/backend/web/chat/dto/response/ChatRoomCreateResponse.java (1)
1-11: 간결하고 적절한 응답 DTO입니다.
ChatRoomCreateResponse가 목적에 맞게 간결하게 구현되었습니다. @builder 패턴 사용으로 객체 생성의 유연성도 확보되어 있습니다.src/main/java/hanium/modic/backend/web/chat/dto/dto/ChatMessageDto.java (1)
1-25: ChatMessageDto ID 타입 검토 결과
ChatMessageDto에서
- messageId: UUID 문자열
- roomId: 채팅방 ID 문자열
- senderId: 사용자 ID(Long)
이렇게 분리된 타입 사용은 WebSocket 페이로드 및 도메인 ID 표현 방식에 맞춘 의도적인 설계로, 불일치가 아닌 올바른 구현입니다. 추가 수정이 필요 없습니다.
src/main/java/hanium/modic/backend/domain/chat/repository/ChatRoomRepository.java (2)
17-20: 활성 채팅방 조회 쿼리가 올바르게 구현되었습니다.soft delete 로직을 고려한 활성 채팅방 조회 쿼리가 정확합니다. 사용자가 user1 또는 user2일 때 각각의 삭제 플래그를 확인하는 로직이 적절합니다.
22-25: 두 사용자 간 채팅방 검색 쿼리가 효율적으로 구현되었습니다.사용자 순서에 관계없이 두 사용자 간의 채팅방을 찾는 쿼리 로직이 정확합니다. OR 조건을 사용하여 (user1, user2)와 (user2, user1) 모든 경우를 처리하고 있습니다.
src/main/java/hanium/modic/backend/web/chat/dto/response/GetMessagesResponse.java (1)
7-13: 레코드 구조가 잘 설계되었습니다.채팅 메시지 응답을 위한 필드들이 적절하게 정의되어 있고, 불변성을 보장하는 record 구문을 사용한 것이 좋습니다.
src/main/java/hanium/modic/backend/common/error/ErrorCode.java (1)
80-84: 채팅 관련 에러 코드가 잘 정의되었습니다.새로 추가된 채팅 에러 코드들이 기존 패턴을 잘 따르고 있으며, 적절한 HTTP 상태 코드와 설명 메시지를 가지고 있습니다. 채팅 기능의 주요 에러 시나리오들이 잘 커버되어 있습니다.
src/main/java/hanium/modic/backend/web/chat/dto/request/MarkMessagesAsReadRequest.java (1)
14-15: String 필드에 대한 검증 어노테이션이 올바르게 사용되었습니다.
lastReadMessageIdString 필드에@NotBlank를 사용한 것이 적절합니다.src/main/java/hanium/modic/backend/web/chat/dto/response/GetChatRoomsResponse.java (2)
8-16: 채팅방 목록 응답 DTO가 잘 설계되었습니다.필요한 모든 정보가 적절한 타입으로 포함되어 있고, Builder 패턴과 중첩된 OpponentDto 클래스를 통해 깔끔한 구조를 유지하고 있습니다.
18-24: 중첩된 OpponentDto 클래스가 적절하게 구현되었습니다.상대방 정보를 캡슐화하는 정적 중첩 클래스로 잘 설계되었으며, 필요한 사용자 정보를 적절한 타입으로 포함하고 있습니다.
src/main/java/hanium/modic/backend/web/chat/controller/ChatWebSocketController.java (1)
25-41: 메시지 전송 메서드가 잘 구현되었습니다.Principal을 통한 인증, 서비스 레이어로의 비즈니스 로직 위임, SimpMessagingTemplate을 통한 브로드캐스팅이 적절하게 구현되어 있습니다. 예외 처리도 적절합니다.
src/main/java/hanium/modic/backend/common/websocket/config/WebSocketConfig.java (1)
33-33: 보안 취약점: CORS 설정 – 허용 출처 제한 필요
현재WebSocketConfig.java에서.setAllowedOriginPatterns("*") // TODO: 실제 도메인으로 변경 필요로 모든 출처를 허용하고 있습니다. 프로덕션 환경에서는 신뢰할 수 있는 도메인만 허용해야 합니다.
점검 항목:
- 파일:
src/main/java/hanium/modic/backend/common/websocket/config/WebSocketConfig.java(33행)registry.addEndpoint("/ws/chat") - .setAllowedOriginPatterns("*") // TODO: 실제 도메인으로 변경 필요 + .setAllowedOriginPatterns(allowedOrigins)- 환경 설정 파일(
application.properties또는application.yml)에websocket.allowed-origins프로퍼티가 정의되어 있는지 확인websocket: allowed-origins: - https://example.com - https://api.example.com@Value("${websocket.allowed-origins}") private String[] allowedOrigins;형태로 바인딩 적용프로덕션에서 사용할 도메인이 외부 설정에 정확히 정의되어 있는지 검증 후 반영 부탁드립니다.
| if (token == null) { | ||
| log.warn("StompChannelInterceptor - STOMP 연결 실패 - Authorization 헤더 누락 또는 형식 오류"); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
인증 실패 시 더 명확한 에러 처리 필요
null을 반환하는 대신 MessageHandlingException을 던져 클라이언트에게 명확한 에러 메시지를 전달하는 것이 좋습니다.
if (token == null) {
log.warn("StompChannelInterceptor - STOMP 연결 실패 - Authorization 헤더 누락 또는 형식 오류");
- return null;
+ throw new MessageHandlingException("인증 토큰이 필요합니다.");
} } catch (Exception e) {
log.warn("StompChannelInterceptor - STOMP 연결 인증 실패: {}", e.getMessage());
- return null;
+ throw new MessageHandlingException("인증에 실패했습니다: " + e.getMessage());
}Also applies to: 62-65
🤖 Prompt for AI Agents
In
src/main/java/hanium/modic/backend/common/websocket/interceptor/StompChannelInterceptor.java
at lines 45-48 and similarly at 62-65, instead of returning null when
authentication fails due to missing or malformed Authorization header, throw a
MessageHandlingException with a clear error message. This change will provide
the client with a precise error notification rather than silently returning
null.
|
|
||
| import static hanium.modic.backend.common.error.ErrorCode.*; | ||
|
|
||
| import java.security.Principal; |
There was a problem hiding this comment.
사용하지 않는 import 제거
사용하지 않는 Principal import와 중복된 ErrorCode import를 제거하세요.
-import java.security.Principal;
import java.util.Map;
import org.springframework.http.server.ServerHttpRequest;
import org.springframework.http.server.ServerHttpResponse;
import org.springframework.web.socket.WebSocketHandler;
import org.springframework.web.socket.server.HandshakeInterceptor;
-import hanium.modic.backend.common.error.ErrorCode;
import hanium.modic.backend.common.error.exception.AppException;Also applies to: 13-13
🤖 Prompt for AI Agents
In
src/main/java/hanium/modic/backend/common/websocket/interceptor/WebSocketAuthInterceptor.java
at lines 5 and 13, remove the unused import of java.security.Principal and the
duplicated import of ErrorCode to clean up the imports and avoid redundancy.
| @Column(name = "message_id", unique = true, nullable = false) | ||
| private String messageId; | ||
|
|
||
| @Column(name = "chatRoom_id", nullable = false) |
There was a problem hiding this comment.
컬럼명 네이밍 일관성 문제
chatRoom_id는 카멜케이스와 스네이크케이스가 혼재되어 있습니다. 다른 컬럼들과 일관성을 유지하기 위해 chat_room_id로 수정하세요.
- @Column(name = "chatRoom_id", nullable = false)
+ @Column(name = "chat_room_id", nullable = false)🤖 Prompt for AI Agents
In src/main/java/hanium/modic/backend/domain/chat/entity/ChatMessageEntity.java
at line 34, the column name "chatRoom_id" mixes camelCase and snake_case,
causing inconsistency. Rename the column to "chat_room_id" to maintain
consistent snake_case naming with other columns.
| @Builder | ||
| private ChatRoomEntity(UserEntity user1, UserEntity user2) { | ||
| this.user1Id = user1.getId(); | ||
| this.user2Id = user2.getId(); | ||
| this.user1Deleted = false; | ||
| this.user2Deleted = false; | ||
| } |
There was a problem hiding this comment.
자기 자신과의 채팅방 생성 방지 필요
Builder에서 같은 사용자끼리 채팅방을 만드는 것을 방지하는 검증이 필요합니다.
@Builder
private ChatRoomEntity(UserEntity user1, UserEntity user2) {
+ if (user1.getId().equals(user2.getId())) {
+ throw new IllegalArgumentException("같은 사용자끼리 채팅방을 만들 수 없습니다.");
+ }
this.user1Id = user1.getId();
this.user2Id = user2.getId();
this.user1Deleted = false;
this.user2Deleted = false;
}🤖 Prompt for AI Agents
In src/main/java/hanium/modic/backend/domain/chat/entity/ChatRoomEntity.java
between lines 39 and 45, add validation in the builder constructor to check if
user1 and user2 are the same user by comparing their IDs. If they are the same,
throw an IllegalArgumentException to prevent creating a chat room with the same
user.
| // 상대방 유저 반환 | ||
| public Long getOpponent(Long userId) { | ||
| if (user1Id.equals(userId)) { | ||
| return user2Id; | ||
| } | ||
| return user1Id; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
getOpponent 메서드 개선 필요
userId가 채팅방 참여자가 아닌 경우에 대한 처리가 필요합니다.
public Long getOpponent(Long userId) {
if (user1Id.equals(userId)) {
return user2Id;
+ } else if (user2Id.equals(userId)) {
+ return user1Id;
+ } else {
+ throw new IllegalArgumentException("사용자가 이 채팅방의 참여자가 아닙니다.");
}
- return user1Id;
}📝 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.
| // 상대방 유저 반환 | |
| public Long getOpponent(Long userId) { | |
| if (user1Id.equals(userId)) { | |
| return user2Id; | |
| } | |
| return user1Id; | |
| } | |
| // 상대방 유저 반환 | |
| public Long getOpponent(Long userId) { | |
| if (user1Id.equals(userId)) { | |
| return user2Id; | |
| } else if (user2Id.equals(userId)) { | |
| return user1Id; | |
| } else { | |
| throw new IllegalArgumentException("사용자가 이 채팅방의 참여자가 아닙니다."); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/hanium/modic/backend/domain/chat/entity/ChatRoomEntity.java
between lines 56 and 62, the getOpponent method lacks handling for cases where
the provided userId is not a participant in the chat room. Modify the method to
check if userId matches either user1Id or user2Id; if it does not, throw an
appropriate exception or return a null/optional value to indicate invalid input,
ensuring the method handles non-participant userIds safely.
| @Query("SELECT cm FROM ChatMessageEntity cm " + | ||
| "WHERE cm.chatRoomId = :chatRoomId " + | ||
| "ORDER BY cm.createAt DESC " + | ||
| "LIMIT 1") | ||
| Optional<ChatMessageEntity> findLatestMessageByChatRoomId(@Param("chatRoomId") Long chatRoomId); |
There was a problem hiding this comment.
JPQL의 LIMIT 절 호환성 문제
JPQL에서 LIMIT 절은 표준이 아니며, 모든 JPA 구현체에서 지원되지 않을 수 있습니다. Hibernate에서는 작동할 수 있지만, 다른 JPA 구현체로 전환 시 문제가 발생할 수 있습니다.
표준 JPA 방식으로 변경하는 것을 권장합니다:
-@Query("SELECT cm FROM ChatMessageEntity cm " +
- "WHERE cm.chatRoomId = :chatRoomId " +
- "ORDER BY cm.createAt DESC " +
- "LIMIT 1")
-Optional<ChatMessageEntity> findLatestMessageByChatRoomId(@Param("chatRoomId") Long chatRoomId);
+Optional<ChatMessageEntity> findTopByChatRoomIdOrderByCreateAtDesc(Long chatRoomId);또는 Pageable을 사용한 대안:
@Query("SELECT cm FROM ChatMessageEntity cm " +
"WHERE cm.chatRoomId = :chatRoomId " +
- "ORDER BY cm.createAt DESC " +
- "LIMIT 1")
-Optional<ChatMessageEntity> findLatestMessageByChatRoomId(@Param("chatRoomId") Long chatRoomId);
+ "ORDER BY cm.createAt DESC")
+List<ChatMessageEntity> findLatestMessageByChatRoomId(@Param("chatRoomId") Long chatRoomId, Pageable pageable);서비스 레이어에서는 다음과 같이 사용:
PageRequest.of(0, 1)🤖 Prompt for AI Agents
In
src/main/java/hanium/modic/backend/domain/chat/repository/ChatMessageRepository.java
around lines 31 to 35, the JPQL query uses a non-standard LIMIT clause which may
not be supported by all JPA implementations. To fix this, remove the LIMIT
clause from the query and instead use Spring Data JPA's Pageable feature by
defining the method to accept a Pageable parameter and return a list or page of
results. Then, in the service layer, call this method with PageRequest.of(0, 1)
to retrieve the latest message. This ensures compatibility across different JPA
providers.
| @NotBlank(message = "채팅방 ID는 필수입니다.") | ||
| private Long chatRoomId; |
There was a problem hiding this comment.
검증 어노테이션이 잘못 사용되었습니다.
@NotBlank는 String 필드에만 사용해야 하며, Long 타입인 chatRoomId에는 @NotNull을 사용해야 합니다.
다음과 같이 수정해주세요:
- @NotBlank(message = "채팅방 ID는 필수입니다.")
+ @NotNull(message = "채팅방 ID는 필수입니다.")
private Long chatRoomId;📝 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.
| @NotBlank(message = "채팅방 ID는 필수입니다.") | |
| private Long chatRoomId; | |
| @NotNull(message = "채팅방 ID는 필수입니다.") | |
| private Long chatRoomId; |
🤖 Prompt for AI Agents
In
src/main/java/hanium/modic/backend/web/chat/dto/request/MarkMessagesAsReadRequest.java
at lines 11-12, the @NotBlank annotation is incorrectly applied to a Long field
chatRoomId. Replace @NotBlank with @NotNull to properly validate that chatRoomId
is not null, as @NotBlank is only for String fields.
| public static GetMessagesResponse from(ChatMessageEntity chatMessage) { | ||
| return new GetMessagesResponse( | ||
| chatMessage.getMessageId(), | ||
| chatMessage.getSenderId(), | ||
| chatMessage.getMessage(), | ||
| chatMessage.getCreateAt(), | ||
| chatMessage.getIsRead() | ||
| ); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
필드명과 엔티티 메서드명 간의 불일치를 확인해주세요.
응답 필드는 sentAt인데 엔티티 메서드는 getCreateAt()을 사용하고 있습니다. 이는 의미적 불일치를 나타낼 수 있습니다.
다음 스크립트를 실행하여 ChatMessageEntity의 메서드명을 확인해주세요:
🏁 Script executed:
#!/bin/bash
# ChatMessageEntity의 시간 관련 필드와 메서드 확인
ast-grep --pattern 'class ChatMessageEntity {
$$$
$FIELD_TYPE $FIELD_NAME;
$$$
}'
# 시간 관련 getter 메서드 확인
rg -A 3 "getCreateAt|getSentAt|getTimestamp" --type javaLength of output: 1807
ChatMessageEntity 클래스 정의에서 createAt 필드와 관련 getter 메서드를 직접 확인해주세요. 아래 스크립트를 실행해 주시면 위치와 내용을 파악할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# ChatMessageEntity 소스 파일 경로 조회
ENTITY_FILE=$(rg -l "class ChatMessageEntity" --type java)
echo "▶ ChatMessageEntity 파일: $ENTITY_FILE"
# 클래스 상단 200줄 출력 (필드·메서드 정의 확인용)
sed -n '1,200p' "$ENTITY_FILE"
# 시간 관련 필드 및 메서드만 추출
echo -e "\n▶ 시간 관련 필드 및 메서드 확인"
rg -n "createAt|getSentAt|sentAt" --color=always "$ENTITY_FILE"Length of output: 2246
ChatMessageEntity가 상속하는 BaseEntity에 createAt 필드와 getter가 어떻게 정의되어 있는지 확인이 필요합니다. 아래 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash
# BaseEntity 파일 경로 조회
BASE_ENTITY_FILE=$(rg -l "class BaseEntity" --type java)
echo "▶ BaseEntity 파일: $BASE_ENTITY_FILE"
# 클래스 상단 200줄 출력 (필드·메서드 정의 확인용)
sed -n '1,200p' "$BASE_ENTITY_FILE"
# 시간 관련 필드 및 메서드만 추출
echo -e "\n▶ 시간 관련 필드 및 메서드 확인"
rg -n "At" --color=always "$BASE_ENTITY_FILE"Length of output: 1364
GetMessagesResponse의 sentAt 필드명과 BaseEntity의 createAt 필드명 일관성 점검 필요
BaseEntity에서 @CreatedDate로 createAt 필드 및 getCreateAt() 메서드가 제공됨을 확인했습니다. 현재 DTO에서는 이 값을 꺼내 sentAt에 할당하고 있어 의미는 통하지만, 네이밍 불일치로 혼동이 발생할 수 있습니다. 일관성을 높이려면 다음 중 하나를 적용하세요:
- DTO 필드명과 생성자 파라미터를
createdAt으로 변경하고, 외부 JSON에만sentAt으로 노출하고 싶다면- private LocalDateTime sentAt; + @JsonProperty("sentAt") + private LocalDateTime createdAt; - public GetMessagesResponse(…, LocalDateTime sentAt, …) { + public GetMessagesResponse(…, LocalDateTime createdAt, …) { this.sentAt = sentAt; + this.createdAt = createdAt; … }
- 또는 엔티티에
getSentAt()alias 메서드를 추가해도 됩니다:public LocalDateTime getSentAt() { return getCreateAt(); }
수정 대상:
- src/main/java/hanium/modic/backend/web/chat/dto/response/GetMessagesResponse.java
- 필요 시 src/main/java/hanium/modic/backend/common/entity/BaseEntity.java (alias 추가)
🤖 Prompt for AI Agents
In
src/main/java/hanium/modic/backend/web/chat/dto/response/GetMessagesResponse.java
around lines 15 to 23, the DTO uses the field name sentAt while the BaseEntity
uses createAt, causing naming inconsistency. To fix this, rename the DTO field
and constructor parameter from sentAt to createdAt to match BaseEntity, and if
you want to expose sentAt in JSON, use a JSON property annotation to map
createdAt to sentAt. Alternatively, add a getSentAt() alias method in BaseEntity
that returns createAt. Apply these changes to ensure consistent naming between
entity and DTO.
| public void sendMessage( | ||
| @DestinationVariable Long roomId, | ||
| @Payload SendMessageRequest message, | ||
| Principal principal |
There was a problem hiding this comment.
여기서 Principal로 가져오신 이유가 있나요? 혹시 여기선 @CurrentUser가 적용 불가능한가용?
2 participants
Summary
주요 구현 사항
🏗️ 데이터베이스 설계
🔒 보안 기능
🌐 REST API
POST /api/chat/rooms/{receiverId}- 채팅방 생성GET /api/chat/rooms- 채팅방 목록 조회GET /api/chat/messages- 메시지 목록 조회 (페이징)POST /api/chat/read- 메시지 읽음 처리DELETE /api/chat/room/{chatRoomId}- 채팅방 나가기🔌 WebSocket API
ws://yourserver.com/ws/chat/app/chat.sendMessage/{roomId}/app/chat.addUser/{roomId}/app/chat.removeUser/{roomId}/topic/chat/{roomId}구독📊 주요 기능
개선사항
Summary by CodeRabbit
신규 기능
버그 수정
문서화