feat: 채팅 메시지 송수신 구현 #423
feat: 채팅 메시지 송수신 구현 #423
Conversation
whqtker
requested review from
Gyuhyeok99,
lsy1307,
nayonsoso and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/chat/config/CustomHandshakeHandler.java (1)
14-26: WebSocket 핸드셰이크 구현이 올바르게 되어 있습니다Principal 추출 로직이 안전하게 구현되어 있고, instanceof 체크와 폴백 처리도 적절합니다.
더 깔끔한 코드를 위한 작은 개선 제안:
@Override protected Principal determineUser(ServerHttpRequest request, WebSocketHandler wsHandler, Map<String, Object> attributes) { - Object userAttribute = attributes.get("user"); - - if (userAttribute instanceof Principal) { - Principal principal = (Principal) userAttribute; - return principal; - } + if (attributes != null && attributes.get("user") instanceof Principal principal) { + return principal; + } return super.determineUser(request, wsHandler, attributes); }src/main/java/com/example/solidconnection/chat/dto/ChatMessageResponse.java (1)
14-17: 정적 팩토리 메서드가 불필요할 수 있습니다.Java record는 이미 모든 필드를 매개변수로 받는 생성자를 자동으로 제공합니다.
of메서드가 단순히 생성자를 호출하는 것 외에 추가적인 로직(유효성 검증, 변환 등)을 제공하지 않는다면 제거하는 것을 고려해보세요.만약 미래에 생성 로직이 복잡해질 가능성이 있다면 유지하되, 현재로서는 다음과 같이 직접 생성자를 사용하는 것도 충분합니다:
// 현재: ChatMessageResponse.of(id, content, senderId, createdAt, attachments) // 제안: new ChatMessageResponse(id, content, senderId, createdAt, attachments)src/main/java/com/example/solidconnection/chat/service/ChatService.java (1)
47-59: 순환 의존성 해결을 위한 아키텍처 개선 제안
@Lazy어노테이션으로 순환 의존성을 해결하신 것은 유효한 접근이지만, 근본적인 아키텍처 개선이 필요할 수 있습니다. 이벤트 기반 아키텍처나 메시지 발행을 담당하는 별도의 컴포넌트를 고려해보시면 좋겠습니다.src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java (1)
303-365: 동시성 테스트 추가 제안현재 테스트는 포괄적이지만, 채팅의 실시간 특성을 고려하여 동시성 시나리오 테스트를 추가하면 좋겠습니다:
- 동시에 여러 사용자가 메시지를 전송하는 경우
- 읽음 상태를 동시에 업데이트하는 경우
동시성 테스트 케이스를 작성해드릴까요?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
src/main/java/com/example/solidconnection/chat/config/CustomHandshakeHandler.java(1 hunks)src/main/java/com/example/solidconnection/chat/config/SiteUserPrincipal.java(1 hunks)src/main/java/com/example/solidconnection/chat/config/StompHandler.java(2 hunks)src/main/java/com/example/solidconnection/chat/config/StompWebSocketConfig.java(1 hunks)src/main/java/com/example/solidconnection/chat/config/WebSocketHandshakeInterceptor.java(1 hunks)src/main/java/com/example/solidconnection/chat/controller/ChatController.java(1 hunks)src/main/java/com/example/solidconnection/chat/controller/ChatMessageController.java(1 hunks)src/main/java/com/example/solidconnection/chat/domain/ChatAttachment.java(1 hunks)src/main/java/com/example/solidconnection/chat/domain/ChatMessage.java(1 hunks)src/main/java/com/example/solidconnection/chat/domain/ChatParticipant.java(1 hunks)src/main/java/com/example/solidconnection/chat/domain/ChatReadStatus.java(1 hunks)src/main/java/com/example/solidconnection/chat/domain/ChatRoom.java(2 hunks)src/main/java/com/example/solidconnection/chat/dto/ChatAttachmentResponse.java(1 hunks)src/main/java/com/example/solidconnection/chat/dto/ChatMessageResponse.java(1 hunks)src/main/java/com/example/solidconnection/chat/dto/ChatMessageSendRequest.java(1 hunks)src/main/java/com/example/solidconnection/chat/dto/ChatMessageSendResponse.java(1 hunks)src/main/java/com/example/solidconnection/chat/dto/ChatParticipantResponse.java(1 hunks)src/main/java/com/example/solidconnection/chat/dto/ChatRoomListResponse.java(1 hunks)src/main/java/com/example/solidconnection/chat/dto/ChatRoomResponse.java(1 hunks)src/main/java/com/example/solidconnection/chat/repository/ChatAttachmentRepository.java(1 hunks)src/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.java(1 hunks)src/main/java/com/example/solidconnection/chat/repository/ChatParticipantRepository.java(1 hunks)src/main/java/com/example/solidconnection/chat/repository/ChatReadStatusRepository.java(1 hunks)src/main/java/com/example/solidconnection/chat/repository/ChatRoomRepository.java(1 hunks)src/main/java/com/example/solidconnection/chat/service/ChatService.java(1 hunks)src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(2 hunks)src/main/java/com/example/solidconnection/security/config/SecurityConfiguration.java(2 hunks)src/test/java/com/example/solidconnection/chat/fixture/ChatAttachmentFixture.java(1 hunks)src/test/java/com/example/solidconnection/chat/fixture/ChatAttachmentFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/chat/fixture/ChatMessageFixture.java(1 hunks)src/test/java/com/example/solidconnection/chat/fixture/ChatMessageFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/chat/fixture/ChatParticipantFixture.java(1 hunks)src/test/java/com/example/solidconnection/chat/fixture/ChatParticipantFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/chat/fixture/ChatReadStatusFixture.java(1 hunks)src/test/java/com/example/solidconnection/chat/fixture/ChatReadStatusFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/chat/fixture/ChatRoomFixture.java(1 hunks)src/test/java/com/example/solidconnection/chat/fixture/ChatRoomFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/chat/repository/ChatReadStatusRepositoryForTest.java(1 hunks)src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: sockjs configuration should be added to stomp endpoints using .withsockjs() for better browser compa...
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#400
File: src/main/java/com/example/solidconnection/chat/config/StompWebSocketConfig.java:20-22
Timestamp: 2025-07-23T18:07:08.348Z
Learning: SockJS configuration should be added to STOMP endpoints using .withSockJS() for better browser compatibility and network environment support, even if frontend doesn't initially use SockJS client libraries.
Applied to files:
src/main/java/com/example/solidconnection/chat/config/StompWebSocketConfig.javasrc/main/java/com/example/solidconnection/chat/config/StompHandler.java
📚 Learning: 사용자 whqtker는 jpa save() 메서드를 사용한 upsert 연산에서 레이스 컨디션 문제를 우려하고 있으며, 특히 채팅 읽음 상태 업데이트와 같이 동시성이 중요한 연산에...
Learnt from: whqtker
PR: solid-connection/solid-connect-server#408
File: src/main/java/com/example/solidconnection/chat/repository/ChatReadStatusRepository.java:11-17
Timestamp: 2025-07-29T17:26:08.811Z
Learning: 사용자 whqtker는 JPA save() 메서드를 사용한 upsert 연산에서 레이스 컨디션 문제를 우려하고 있으며, 특히 채팅 읽음 상태 업데이트와 같이 동시성이 중요한 연산에서는 네이티브 쿼리의 원자적 연산이 더 안전하다고 인식하고 있습니다.
Applied to files:
src/main/java/com/example/solidconnection/chat/repository/ChatReadStatusRepository.java
🧬 Code Graph Analysis (7)
src/main/java/com/example/solidconnection/chat/controller/ChatMessageController.java (2)
src/main/java/com/example/solidconnection/security/authentication/TokenAuthentication.java (1)
TokenAuthentication(7-42)src/main/java/com/example/solidconnection/security/userdetails/SiteUserDetails.java (1)
SiteUserDetails(9-56)
src/test/java/com/example/solidconnection/chat/fixture/ChatMessageFixture.java (7)
src/test/java/com/example/solidconnection/chat/fixture/ChatParticipantFixture.java (1)
TestComponent(8-20)src/test/java/com/example/solidconnection/chat/fixture/ChatMessageFixtureBuilder.java (1)
TestComponent(9-42)src/test/java/com/example/solidconnection/chat/fixture/ChatParticipantFixtureBuilder.java (1)
TestComponent(9-36)src/test/java/com/example/solidconnection/chat/fixture/ChatReadStatusFixtureBuilder.java (1)
TestComponent(8-35)src/test/java/com/example/solidconnection/chat/fixture/ChatRoomFixtureBuilder.java (1)
TestComponent(8-29)src/test/java/com/example/solidconnection/chat/fixture/ChatRoomFixture.java (1)
TestComponent(7-18)src/test/java/com/example/solidconnection/chat/fixture/ChatReadStatusFixture.java (1)
TestComponent(7-19)
src/test/java/com/example/solidconnection/chat/fixture/ChatReadStatusFixture.java (4)
src/test/java/com/example/solidconnection/chat/fixture/ChatMessageFixture.java (1)
TestComponent(8-21)src/test/java/com/example/solidconnection/chat/fixture/ChatParticipantFixture.java (1)
TestComponent(8-20)src/test/java/com/example/solidconnection/chat/fixture/ChatReadStatusFixtureBuilder.java (1)
TestComponent(8-35)src/test/java/com/example/solidconnection/chat/fixture/ChatRoomFixture.java (1)
TestComponent(7-18)
src/main/java/com/example/solidconnection/chat/config/StompHandler.java (2)
src/main/java/com/example/solidconnection/security/authentication/TokenAuthentication.java (1)
TokenAuthentication(7-42)src/main/java/com/example/solidconnection/security/userdetails/SiteUserDetails.java (1)
SiteUserDetails(9-56)
src/test/java/com/example/solidconnection/chat/fixture/ChatParticipantFixtureBuilder.java (2)
src/test/java/com/example/solidconnection/chat/fixture/ChatParticipantFixture.java (1)
TestComponent(8-20)src/test/java/com/example/solidconnection/chat/fixture/ChatRoomFixtureBuilder.java (1)
TestComponent(8-29)
src/test/java/com/example/solidconnection/chat/fixture/ChatAttachmentFixture.java (1)
src/test/java/com/example/solidconnection/chat/fixture/ChatAttachmentFixtureBuilder.java (1)
TestComponent(9-48)
src/test/java/com/example/solidconnection/chat/fixture/ChatAttachmentFixtureBuilder.java (1)
src/test/java/com/example/solidconnection/chat/fixture/ChatAttachmentFixture.java (1)
TestComponent(8-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (52)
src/main/java/com/example/solidconnection/chat/domain/ChatReadStatus.java (1)
36-39: 깔끔한 생성자 구현이네요!채팅방 ID와 참여자 ID를 받는 생성자가 명확하고 간단하게 구현되어 있습니다. primitive 타입을 사용해서 null 체크도 불필요하고, 필드 초기화도 정확합니다.
src/main/java/com/example/solidconnection/chat/repository/ChatAttachmentRepository.java (1)
6-6: 표준적인 JPA 레포지토리 구현입니다
JpaRepository를 올바르게 확장하고 있고, 제네릭 타입도 정확합니다. 기본적인 CRUD 작업에는 충분한 구현이네요.src/main/java/com/example/solidconnection/chat/config/SiteUserPrincipal.java (1)
5-11: WebSocket 인증을 위한 깔끔한 Principal 구현이네요
Principal인터페이스를 올바르게 구현했고,getName()에서 이메일을 반환하는 것도 적절합니다. 사용자 식별에 필요한id와src/main/java/com/example/solidconnection/security/config/SecurityConfiguration.java (1)
65-65: WebSocket 보안을 위한 적절한 인증 규칙 추가
/connect/**엔드포인트에 인증을 요구하는 규칙을 추가한 것이 채팅 기능의 보안을 위해 꼭 필요한 변경사항입니다. WebSocket 핸드셰이크 시점에서 사용자 인증을 보장할 수 있겠네요.src/main/java/com/example/solidconnection/chat/domain/ChatMessage.java (4)
37-37: 컬렉션 불변성 보장이 잘 되었습니다
chatAttachments필드를final로 선언하여 참조 불변성을 보장한 점이 좋습니다. 이렇게 하면 컬렉션 자체의 재할당을 방지하면서도 내부 요소 조작은 여전히 가능하게 됩니다.
39-46: 양방향 연관관계 관리가 깔끔하게 구현되었습니다새로 추가된 생성자가 다음과 같이 잘 설계되었습니다:
- 필수 필드 초기화:
content,senderId,chatRoom필드를 명확하게 설정- 양방향 연관관계 자동 관리:
chatRoom이 null이 아닐 때 자동으로 부모 컬렉션에 추가- NPE 방지: null 체크로 안전성 확보
이 패턴은 다른 도메인 엔티티들과 일관성을 유지하며, 도메인 객체 생성 시 연관관계 누락을 방지하는 좋은 접근법입니다.
37-37: 불변성 개선이 좋습니다!
chatAttachments필드를final로 선언하여 컬렉션 참조의 불변성을 보장한 점이 훌륭합니다. 이는 의도치 않은 컬렉션 재할당을 방지하고 엔티티의 안정성을 높입니다.
39-46: 양방향 연관관계 관리가 잘 구현되었습니다!
- 생성자 파라미터:
content,senderId,chatRoom을 받아 핵심 필드를 초기화- 방어적 프로그래밍:
chatRoom != null체크로 NPE 방지- 자동 연관관계 설정: 생성과 동시에 부모 엔티티의 컬렉션에 자동 추가
이 패턴은 도메인 일관성을 유지하고 개발자의 실수를 줄여주는 좋은 설계입니다.
src/main/java/com/example/solidconnection/chat/dto/ChatRoomListResponse.java (2)
5-12: 간결하고 목적에 맞는 DTO 설계입니다채팅방 목록 응답을 위한 record 클래스가 다음과 같이 잘 구현되었습니다:
- 불변 데이터 구조: record를 사용하여 immutable DTO 구현
- 명확한 팩토리 메서드:
of()메서드로 생성 의도를 명확히 표현- 단순한 래퍼 구조: 채팅방 목록을 감싸는 최소한의 구조로 API 일관성 확보
REST API 응답 구조로 적합하며, 향후 메타데이터 추가 시에도 확장하기 용이한 구조입니다.
5-12: 깔끔한 DTO 설계입니다!
- Record 활용: 불변 데이터 구조로 간결하게 구현
- 정적 팩토리 메서드:
of()메서드로 명확한 인스턴스 생성- 타입 안전성:
List<ChatRoomResponse>제네릭으로 타입 보장채팅방 목록을 감싸는 래퍼 클래스로서 역할이 명확하고, API 응답 구조를 일관성 있게 유지하는 좋은 패턴입니다.
src/main/java/com/example/solidconnection/chat/domain/ChatParticipant.java (2)
38-44: 일관된 양방향 연관관계 패턴 적용이 훌륭합니다
ChatParticipant생성자가 다른 도메인 엔티티들과 동일한 패턴으로 구현되어 일관성이 좋습니다:
- 자동 연관관계 설정: 참가자 생성 시 채팅방의 참가자 목록에 자동 추가
- 안전한 null 처리:
chatRoomnull 체크로 예외 상황 방지- 도메인 로직 캡슐화: 연관관계 관리를 생성자에서 처리하여 비즈니스 로직 단순화
이런 패턴은 참가자 생성 시 연관관계 누락을 방지하고, 도메인 객체의 일관성을 보장하는 좋은 접근법입니다.
38-44: 일관된 패턴으로 양방향 연관관계를 잘 관리하고 있습니다!
- 필드 초기화:
siteUserId와chatRoom핵심 데이터 설정- 안전한 연관관계: null 체크 후 부모 컬렉션에 자동 추가
- 패턴 일관성:
ChatMessage와 동일한 양방향 관리 방식 적용이런 일관된 생성자 패턴은 도메인 모델의 무결성을 보장하고, 다른 개발자들이 코드를 이해하기 쉽게 만듭니다.
src/main/java/com/example/solidconnection/chat/domain/ChatRoom.java (7)
15-15: 성능 최적화를 위한 Hibernate 의존성 추가
@BatchSize어노테이션 사용을 위해 Hibernate 임포트가 추가되었습니다. 이는 성능 최적화를 위한 합리적인 선택입니다.
29-30: N+1 문제 해결을 위한 배치 로딩 최적화
@BatchSize(size = 10)어노테이션을 통해 채팅 참가자 조회 시 배치 로딩을 활용한 것이 좋습니다. 이는 다음과 같은 이점을 제공합니다:
- N+1 쿼리 문제 완화: 참가자 목록 조회 시 개별 쿼리 대신 배치로 처리
- 적절한 배치 크기: 10개 단위로 메모리 사용량과 성능의 균형점 확보
채팅방 목록 조회 시 성능 향상에 도움이 될 것입니다.
30-33: 컬렉션 불변성 보장이 일관되게 적용되었습니다두 컬렉션 모두
final로 선언하여 참조 불변성을 확보한 점이 좋습니다:
- chatParticipants: 참가자 목록의 참조 불변성 보장
- chatMessages: 메시지 목록의 참조 불변성 보장
이는 다른 도메인 엔티티들과 일관된 패턴이며, 컬렉션 재할당으로 인한 예기치 않은 문제를 방지합니다.
35-37: 그룹 채팅 지원을 위한 생성자 추가
isGroup매개변수를 받는 생성자가 추가되어 채팅방 생성 시 그룹 여부를 명시적으로 설정할 수 있게 되었습니다. 이는 1:1 채팅과 그룹 채팅을 구분하는 비즈니스 로직에 필요한 기능입니다.생성자가 단순하고 명확하여 사용하기 쉽습니다.
29-30: 성능 최적화와 불변성 개선이 훌륭합니다!
- 배치 페칭:
@BatchSize(size = 10)로 N+1 쿼리 문제 해결- 컬렉션 불변성:
final키워드로 참조 재할당 방지참가자 컬렉션 조회 시 성능이 크게 개선될 것이며, 동시에 엔티티의 안정성도 보장됩니다.
33-33: 메시지 컬렉션 불변성 보장이 좋습니다!
chatMessages컬렉션도final로 선언하여 일관된 불변성 정책을 적용한 점이 훌륭합니다.
35-37: 명시적인 생성자로 의도가 명확해졌습니다!
isGroup파라미터를 받는 생성자를 추가하여 채팅방 생성 시 그룹 여부를 명시적으로 설정할 수 있게 된 점이 좋습니다. 이는 도메인 객체의 의미를 더욱 명확하게 만듭니다.src/main/java/com/example/solidconnection/chat/dto/ChatParticipantResponse.java (2)
3-12: 채팅 참가자 정보를 위한 명확한 DTO 설계채팅 참가자 응답 DTO가 다음과 같이 깔끔하게 구현되었습니다:
- 필수 정보 포함:
partnerId,nickname,profileUrl로 UI 표시에 필요한 핵심 정보 제공- 불변 구조: record 사용으로 데이터 무결성 보장
- 일관된 팩토리 패턴: 다른 DTO들과 동일한
of()메서드 패턴 적용채팅 참가자 목록 표시나 채팅방 정보 조회 시 활용하기 적합한 구조입니다.
3-12: 채팅 참가자 정보를 깔끔하게 표현한 DTO입니다!
- 명확한 필드명:
partnerId,nickname,profileUrl로 의미가 분명- Record 활용: 불변 데이터 구조로 안전하고 간결함
- 팩토리 메서드:
of()메서드로 일관된 생성 패턴 제공채팅 참가자의 핵심 정보를 담는 DTO로서 역할이 명확하고, 다른 채팅 관련 DTO들과 일관된 설계 패턴을 따르고 있습니다.
src/main/java/com/example/solidconnection/chat/domain/ChatAttachment.java (1)
36-44: 양방향 연관관계 설정이 잘 구현되었습니다생성자에서
ChatMessage와의 양방향 연관관계를 적절히 설정하고 있고, null 체크를 통해 안전성도 확보했습니다. JPA 엔티티 설계 모범 사례를 잘 따르고 있네요.src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
48-49: 채팅 기능용 에러 코드 추가가 적절합니다
- 채팅 상대 관련 에러 코드들이 적절한 HTTP 상태 코드와 함께 추가되었습니다
- 기존 패턴과 일관성 있는 한국어 메시지가 설정되어 있습니다
- 에러 코드 네이밍 규칙을 잘 따르고 있습니다
Also applies to: 125-125
src/main/java/com/example/solidconnection/chat/dto/ChatMessageSendResponse.java (1)
5-18: 깔끔한 DTO 설계입니다
- Record를 활용한 현대적이고 간결한 구현
- 정적 팩토리 메서드 패턴을 적절히 사용
- 도메인 엔티티에서 필요한 필드만 추출하는 명확한 책임
- 불변성이 보장되는 안전한 설계
src/test/java/com/example/solidconnection/chat/fixture/ChatMessageFixture.java (1)
1-21: 테스트 픽스처 패턴이 일관성 있게 잘 구현되었습니다!빌더 패턴을 활용한 테스트 데이터 생성 방식이 다른 픽스처 클래스들과 동일한 구조로 구현되어 있어 코드의 일관성이 뛰어납니다.
src/main/java/com/example/solidconnection/chat/config/StompWebSocketConfig.java (1)
25-26: 새로운 핸드셰이크 구성 요소 추가가 적절합니다
WebSocketHandshakeInterceptor와CustomHandshakeHandler의존성 주입이 깔끔하게 구현되었습니다.src/test/java/com/example/solidconnection/chat/fixture/ChatRoomFixture.java (1)
1-18: 채팅방 픽스처 구현이 표준 패턴을 잘 따르고 있습니다다른 픽스처 클래스들과 동일한 구조로 구현되어 테스트 코드의 일관성을 유지하고 있습니다.
src/test/java/com/example/solidconnection/chat/fixture/ChatReadStatusFixture.java (1)
1-19: 읽음 상태 픽스처가 깔끔하게 구현되었습니다필요한 매개변수들(chatRoomId, chatParticipantId)을 적절히 받아서 빌더 패턴으로 위임하는 구조가 잘 설계되어 있습니다.
src/test/java/com/example/solidconnection/chat/fixture/ChatParticipantFixture.java (1)
1-20: 참여자 픽스처가 도메인 관계를 잘 반영하여 구현되었습니다사용자 ID와 채팅방 엔티티를 매개변수로 받는 구조가 도메인 모델의 관계를 적절히 표현하고 있으며, 다른 픽스처들과 일관된 패턴을 유지하고 있습니다.
src/main/java/com/example/solidconnection/chat/dto/ChatAttachmentResponse.java (1)
5-17: 채팅 첨부파일 응답 DTO가 깔끔하게 구현되었습니다!
- 적절한 필드 구성: 첨부파일에 필요한 핵심 정보들(ID, 이미지 여부, URL, 썸네일 URL, 생성시간)이 모두 포함되어 있어요
- 타입 안전성: ZonedDateTime을 사용해서 시간대 정보까지 포함한 안전한 타임스탬프 처리가 되고 있네요
- 편의성: 정적 팩토리 메서드
of를 제공해서 객체 생성이 명시적이고 가독성이 좋습니다src/main/java/com/example/solidconnection/chat/repository/ChatReadStatusRepository.java (1)
11-17: 읽음 상태 upsert 구현이 잘 되어있습니다!
- 원자적 연산: MySQL의
ON DUPLICATE KEY UPDATE를 사용해서 레이스 컨디션 없이 안전하게 읽음 상태를 업데이트할 수 있어요- 정밀한 타임스탬프:
NOW(6)로 마이크로초 단위까지 정확한 시간 기록이 가능하네요- 적절한 애노테이션:
@Modifying에clearAutomatically와flushAutomatically옵션이 올바르게 설정되어 있습니다네이티브 쿼리 선택이 이런 동시성이 중요한 채팅 읽음 상태 관리에는 최적의 방법이라고 생각해요.
src/main/java/com/example/solidconnection/chat/repository/ChatParticipantRepository.java (1)
7-12: 채팅 참여자 레포지토리가 깔끔하게 설계되었습니다!
- 명확한 메서드 네이밍: Spring Data JPA 컨벤션을 잘 따라서 메서드 이름만으로도 기능을 쉽게 이해할 수 있어요
- 적절한 복합키 활용:
chatRoomId와siteUserId를 조합한 조회가 채팅 참여자 검증에 딱 맞는 방식이네요- 효율적인 메서드 구성: 존재 여부 확인(
exists)과 실제 조회(find) 메서드를 별도로 제공해서 용도에 따른 최적화가 가능합니다src/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.java (1)
13-22: 채팅 메시지 레포지토리가 성능을 고려해서 잘 구현되었습니다!
- N+1 문제 해결:
LEFT JOIN FETCH cm.chatAttachments로 첨부파일까지 한 번에 로딩해서 추가 쿼리를 방지했어요- 효율적인 페이징:
Slice를 사용해서 전체 개수 조회 없이 다음 페이지 존재 여부만 확인하는 방식이 채팅에 적합하네요- 적절한 정렬: 최신 메시지부터 보여주는
ORDER BY cm.createdAt DESC가 채팅 UI에 맞는 순서입니다- 명확한 쿼리 구조: JPQL이 가독성 좋게 멀티라인으로 정리되어 있어서 유지보수가 쉬울 것 같아요
src/main/java/com/example/solidconnection/chat/dto/ChatRoomResponse.java (1)
5-22: 채팅방 응답 DTO가 채팅 리스트 화면에 필요한 정보를 잘 담고 있습니다!
- 완성도 높은 필드 구성: 채팅방 ID, 마지막 메시지, 받은 시간, 상대방 정보, 읽지 않은 메시지 수까지 채팅방 목록에 필요한 모든 정보가 포함되어 있어요
- 사용자 경험 고려:
unReadCount로 읽지 않은 메시지 수를 제공해서 UI에서 뱃지 표시가 가능하네요- 관계 정보 표현:
ChatParticipantResponse partner로 채팅 상대방 정보를 명확하게 표현했습니다- 일관된 팩토리 패턴: 다른 DTO들과 동일하게
of메서드를 제공해서 일관성이 좋아요src/main/java/com/example/solidconnection/chat/controller/ChatMessageController.java (1)
21-30: 메서드 구조가 깔끔하고 단일 책임 원칙을 잘 따르고 있습니다.
- 📝 명확한 매핑:
@MessageMapping("/chat/{roomId}")를 통해 채팅방별 메시지 라우팅이 직관적입니다.- 🔐 인증 처리: Principal에서 사용자 정보를 추출하여 서비스 레이어로 전달하는 구조가 적절합니다.
- 🎯 책임 분리: 컨트롤러는 매개체 역할만 하고 실제 비즈니스 로직은 ChatService에 위임하는 것이 좋은 설계입니다.
src/main/java/com/example/solidconnection/chat/dto/ChatMessageResponse.java (1)
6-12: DTO 구조가 잘 설계되었습니다.
- 📋 적절한 필드 구성: 메시지의 핵심 정보(id, content, senderId, createdAt, attachments)가 모두 포함되어 있습니다.
- ⏰ 타임스탬프 처리:
ZonedDateTime을 사용하여 시간대 정보를 포함한 정확한 시간 표현이 가능합니다.- 🔒 불변성: record를 사용하여 데이터의 불변성을 보장하는 것이 좋은 선택입니다.
src/test/java/com/example/solidconnection/chat/fixture/ChatRoomFixtureBuilder.java (1)
25-28: 엔티티 생성 및 저장 로직이 적절합니다.
- 🏗️ 간단한 생성:
ChatRoom생성자에isGroup플래그를 전달하여 적절히 초기화하고 있습니다.- 💾 즉시 저장: 테스트에서 바로 사용할 수 있도록 repository를 통해 저장 후 반환하는 것이 좋습니다.
src/main/java/com/example/solidconnection/chat/config/StompHandler.java (3)
59-62: 경로 파싱 로직이 개선되었습니다.
- 📍 명확한 토픽 구조:
/topic/chat/{roomId}형태로 경로 구조가 더 명확해졌습니다.- 🔍 엄격한 검증:
parts.length < 4와parts[1].equals("topic"),parts[2].equals("chat")검증을 통해 올바른 형식의 경로만 허용합니다.- 🎯 정확한 추출:
parts[3]에서 roomId를 추출하는 로직이 새로운 경로 구조와 일치합니다.
29-34: CONNECT 명령어 인증 처리가 간소화되었습니다.Principal 기반 인증으로 변경되면서 이전의 복잡한 JWT 파싱 로직이 제거되고 깔끔해졌습니다.
accessor.getUser()를 통해 간단히 인증 상태를 확인할 수 있어 코드의 가독성과 유지보수성이 향상되었습니다.
48-48: 채팅방 참여자 검증이 적절히 위임되었습니다.
ChatService.validateChatRoomParticipant()메서드를 통해 비즈니스 로직을 서비스 계층에 위임하는 것이 좋은 아키텍처 설계입니다. STOMP 핸들러는 프로토콜 처리에만 집중하고 도메인 로직은 서비스에서 처리하도록 책임이 명확히 분리되었습니다.src/main/java/com/example/solidconnection/chat/config/WebSocketHandshakeInterceptor.java (2)
15-26: WebSocket 핸드셰이크 인증 로직이 깔끔하게 구현되었습니다.
- 🔐 간단한 인증 확인:
request.getPrincipal()로 HTTP 요청에서 인증된 사용자 정보를 추출합니다.- 💾 세션 저장: Principal이 존재할 경우
"user"키로 WebSocket 세션 속성에 저장하여 후속 STOMP 메시지에서 활용할 수 있게 합니다.- 🚪 접근 제어: 인증되지 않은 사용자의 WebSocket 연결을 명확히 거부합니다.
- 🎯 단일 책임: 클래스명과 주석에서 명시한 대로 Principal 저장에만 집중하는 깔끔한 설계입니다.
28-31: afterHandshake 메서드 구현이 적절합니다.현재 단계에서는 핸드셰이크 완료 후 추가 작업이 필요하지 않으므로 빈 구현이 적절합니다. 필요에 따라 향후 로깅이나 세션 정리 로직을 추가할 수 있는 확장 지점으로 남겨둔 것이 좋습니다.
src/test/java/com/example/solidconnection/chat/fixture/ChatAttachmentFixture.java (1)
8-30: 테스트 픽스처 구조가 잘 설계되었습니다
- 일관된 네이밍: 한국어 메서드명이 다른 픽스처들과 일관성을 유지하고 있어요
- 편의 메서드 제공:
이미지와파일메서드가 공통 사용 케이스에 대한 편리한 접근을 제공합니다- 빌더 패턴 활용: ChatAttachmentFixtureBuilder를 적절히 활용하여 유연한 테스트 데이터 생성이 가능해요
src/main/java/com/example/solidconnection/chat/repository/ChatRoomRepository.java (1)
9-36: JPQL 쿼리가 잘 구성되었습니다
적절한 쿼리 구조:
findOneOnOneChatRoomsByUserId는 일대일 채팅방 조회에 필요한 조인과 정렬을 정확히 구현했어요NULLS LAST를 사용해 메시지가 없는 채팅방을 마지막에 배치하는 것도 좋은 접근입니다복잡한 읽지 않은 메시지 카운트 로직:
countUnreadMessages의 LEFT JOIN과 서브쿼리 조합이 올바르게 구현되어 있습니다- 사용자가 보내지 않은 메시지 중 읽음 상태가 없거나 메시지 생성 시간이 읽음 시간보다 늦은 것을 정확히 필터링하고 있어요
성능 고려사항: 복잡한 쿼리이므로 관련 인덱스 설정을 확인해보시기 바랍니다
src/test/java/com/example/solidconnection/chat/fixture/ChatParticipantFixtureBuilder.java (2)
18-20: 팩토리 메서드 동작에 대한 확인
chatParticipant()메서드가 새로운 인스턴스를 생성하는 것은 다른 픽스처 빌더들과 일관된 패턴이에요. 다만 현재 빌더의 상태를 복사하지 않으므로, 체이닝 사용 시 매번 새로운 상태로 시작한다는 점을 유의하세요.
32-35: 빌더 패턴 구현이 적절합니다
- 도메인 객체 생성: ChatParticipant 생성자를 올바르게 호출하고 있어요
- 영속화 처리: 레포지토리를 통한 저장 후 반환하는 패턴이 정확합니다
- 일관성: 다른 픽스처 빌더들과 동일한 구조를 유지하고 있어 좋습니다
src/test/java/com/example/solidconnection/chat/fixture/ChatMessageFixtureBuilder.java (1)
9-42: 일관된 빌더 패턴 구현이 우수합니다
- 패턴 일관성: 다른 픽스처 빌더들과 동일한 구조를 유지하여 코드 가독성과 유지보수성이 높아요
- 플루언트 인터페이스: 메서드 체이닝을 통한 직관적인 객체 생성이 가능합니다
- 적절한 필드 구성: ChatMessage 도메인에 필요한 핵심 필드들(content, senderId, chatRoom)을 모두 포함하고 있어요
- 영속화 로직: create() 메서드에서 도메인 객체 생성과 저장을 한 번에 처리하는 것이 테스트 편의성을 높입니다
src/test/java/com/example/solidconnection/chat/fixture/ChatReadStatusFixtureBuilder.java (1)
8-35: 읽음 상태 빌더가 적절히 구현되었습니다
- ID 기반 접근: ChatReadStatus의 특성에 맞게 엔티티 참조 대신 ID 필드를 사용하는 것이 적절해요
- 빌더 패턴 일관성: 다른 픽스처 빌더들과 동일한 구조를 유지하여 전체 테스트 인프라의 일관성을 보장합니다
- 도메인 로직 반영: ChatReadStatus(chatRoomId, chatParticipantId) 생성자를 올바르게 활용하고 있어요
전체적으로 채팅 도메인의 테스트 픽스처 인프라가 체계적으로 잘 구성되어 있습니다.
src/main/java/com/example/solidconnection/chat/service/ChatService.java (2)
157-162: 검증 로직이 잘 구현되었습니다!채팅방 참여자 검증을 별도 메서드로 분리하여 재사용성을 높인 좋은 설계입니다.
130-137: 읽음 처리 로직이 효율적으로 구현되었습니다!Upsert 방식을 사용하여 읽음 상태를 효율적으로 처리하는 좋은 구현입니다.
src/main/java/com/example/solidconnection/chat/controller/ChatController.java (1)
34-42: 페이징 처리가 잘 구현되었습니다!적절한 기본값(20개, 최신순)과 함께 페이징이 잘 구현되어 있습니다. RESTful 규칙에 맞는 좋은 설계입니다.
src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java (1)
37-74: 테스트 구조가 잘 조직되어 있습니다!
@Nested클래스를 사용하여 테스트를 논리적으로 그룹화하고, 명확한 한글 메서드명을 사용한 점이 좋습니다. 테스트 가독성이 뛰어납니다.
| TokenAuthentication tokenAuthentication = (TokenAuthentication) user; | ||
| SiteUserDetails siteUserDetails = (SiteUserDetails) tokenAuthentication.getPrincipal(); |
There was a problem hiding this comment.
타입 캐스팅 안전성을 개선해주세요.
ChatMessageController와 동일하게, Principal을 TokenAuthentication으로, 그리고 그 안의 principal을 SiteUserDetails로 캐스팅하는 부분에서 타입 안전성이 보장되지 않습니다.
안전한 타입 검증을 추가하세요:
- TokenAuthentication tokenAuthentication = (TokenAuthentication) user;
- SiteUserDetails siteUserDetails = (SiteUserDetails) tokenAuthentication.getPrincipal();
+ if (!(user instanceof TokenAuthentication tokenAuthentication)) {
+ throw new CustomException(AUTHENTICATION_FAILED);
+ }
+
+ Object principalObject = tokenAuthentication.getPrincipal();
+ if (!(principalObject instanceof SiteUserDetails siteUserDetails)) {
+ throw new CustomException(AUTHENTICATION_FAILED);
+ }📝 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.
| TokenAuthentication tokenAuthentication = (TokenAuthentication) user; | |
| SiteUserDetails siteUserDetails = (SiteUserDetails) tokenAuthentication.getPrincipal(); | |
| if (!(user instanceof TokenAuthentication tokenAuthentication)) { | |
| throw new CustomException(AUTHENTICATION_FAILED); | |
| } | |
| Object principalObject = tokenAuthentication.getPrincipal(); | |
| if (!(principalObject instanceof SiteUserDetails siteUserDetails)) { | |
| throw new CustomException(AUTHENTICATION_FAILED); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/chat/config/StompHandler.java
around lines 42 to 43, the current casting of user to TokenAuthentication and
then its principal to SiteUserDetails lacks type safety. Add explicit type
checks using instanceof before casting to ensure the objects are of the expected
types, and handle the cases where they are not to prevent ClassCastException.
src/main/java/com/example/solidconnection/chat/config/StompWebSocketConfig.java
Outdated
Show resolved
Hide resolved
| TokenAuthentication tokenAuthentication = (TokenAuthentication) principal; | ||
| SiteUserDetails siteUserDetails = (SiteUserDetails) tokenAuthentication.getPrincipal(); |
There was a problem hiding this comment.
타입 캐스팅의 안전성을 보장해주세요.
Principal을 TokenAuthentication으로, 그리고 그 안의 principal을 SiteUserDetails로 캐스팅하는 부분에서 ClassCastException이 발생할 가능성이 있습니다. WebSocket 세션에서 예상과 다른 타입의 Principal이 전달될 경우를 대비한 검증 로직이 필요합니다.
다음과 같이 안전한 타입 검증을 추가하는 것을 권장합니다:
- TokenAuthentication tokenAuthentication = (TokenAuthentication) principal;
- SiteUserDetails siteUserDetails = (SiteUserDetails) tokenAuthentication.getPrincipal();
+ if (!(principal instanceof TokenAuthentication tokenAuthentication)) {
+ throw new CustomException(ErrorCode.AUTHENTICATION_FAILED);
+ }
+
+ Object principalObject = tokenAuthentication.getPrincipal();
+ if (!(principalObject instanceof SiteUserDetails siteUserDetails)) {
+ throw new CustomException(ErrorCode.AUTHENTICATION_FAILED);
+ }🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/chat/controller/ChatMessageController.java
around lines 26 to 27, the code casts Principal to TokenAuthentication and then
to SiteUserDetails without type checks, risking ClassCastException. Add
instanceof checks before each cast to verify the object's type, and handle the
case where the type is unexpected, such as by logging an error or throwing a
controlled exception, to ensure safe type casting.
src/main/java/com/example/solidconnection/chat/dto/ChatMessageSendRequest.java
Show resolved
Hide resolved
| @Transactional | ||
| public void sendChatMessage(ChatMessageSendRequest chatMessageSendRequest, long siteUserId, long roomId) { | ||
| validateChatRoomParticipant(siteUserId, roomId); | ||
|
|
||
| ChatMessage chatMessage = new ChatMessage( | ||
| chatMessageSendRequest.content(), | ||
| siteUserId, | ||
| chatRoomRepository.findById(roomId) | ||
| .orElseThrow(() -> new CustomException(INVALID_CHAT_ROOM_STATE)) | ||
| ); | ||
|
|
||
| chatMessageRepository.save(chatMessage); | ||
|
|
||
| ChatMessageSendResponse chatMessageResponse = ChatMessageSendResponse.from(chatMessage); | ||
|
|
||
| simpMessagingTemplate.convertAndSend("/topic/chat/" + roomId, chatMessageResponse); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
메시지 전송 로직 개선 제안
현재 구현은 동작하지만 다음 개선사항을 고려해보세요:
- 상수 사용:
/topic/chat/경로를 상수로 정의 - 에러 처리: 메시징 전송 실패 시 처리 로직 추가
- 트랜잭션 범위: DB 저장과 메시지 전송의 원자성 보장 고려
+private static final String CHAT_TOPIC_PREFIX = "/topic/chat/";
+
@Transactional
public void sendChatMessage(ChatMessageSendRequest chatMessageSendRequest, long siteUserId, long roomId) {
validateChatRoomParticipant(siteUserId, roomId);
ChatMessage chatMessage = new ChatMessage(
chatMessageSendRequest.content(),
siteUserId,
chatRoomRepository.findById(roomId)
.orElseThrow(() -> new CustomException(INVALID_CHAT_ROOM_STATE))
);
chatMessageRepository.save(chatMessage);
ChatMessageSendResponse chatMessageResponse = ChatMessageSendResponse.from(chatMessage);
- simpMessagingTemplate.convertAndSend("/topic/chat/" + roomId, chatMessageResponse);
+ try {
+ simpMessagingTemplate.convertAndSend(CHAT_TOPIC_PREFIX + roomId, chatMessageResponse);
+ } catch (Exception e) {
+ log.error("Failed to send message to WebSocket topic: {}", e.getMessage());
+ // 필요시 보상 트랜잭션 또는 재시도 로직 추가
+ }
}📝 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.
| @Transactional | |
| public void sendChatMessage(ChatMessageSendRequest chatMessageSendRequest, long siteUserId, long roomId) { | |
| validateChatRoomParticipant(siteUserId, roomId); | |
| ChatMessage chatMessage = new ChatMessage( | |
| chatMessageSendRequest.content(), | |
| siteUserId, | |
| chatRoomRepository.findById(roomId) | |
| .orElseThrow(() -> new CustomException(INVALID_CHAT_ROOM_STATE)) | |
| ); | |
| chatMessageRepository.save(chatMessage); | |
| ChatMessageSendResponse chatMessageResponse = ChatMessageSendResponse.from(chatMessage); | |
| simpMessagingTemplate.convertAndSend("/topic/chat/" + roomId, chatMessageResponse); | |
| } | |
| // Define constant for the topic prefix | |
| private static final String CHAT_TOPIC_PREFIX = "/topic/chat/"; | |
| @Transactional | |
| public void sendChatMessage(ChatMessageSendRequest chatMessageSendRequest, long siteUserId, long roomId) { | |
| validateChatRoomParticipant(siteUserId, roomId); | |
| ChatMessage chatMessage = new ChatMessage( | |
| chatMessageSendRequest.content(), | |
| siteUserId, | |
| chatRoomRepository.findById(roomId) | |
| .orElseThrow(() -> new CustomException(INVALID_CHAT_ROOM_STATE)) | |
| ); | |
| chatMessageRepository.save(chatMessage); | |
| ChatMessageSendResponse chatMessageResponse = ChatMessageSendResponse.from(chatMessage); | |
| try { | |
| simpMessagingTemplate.convertAndSend(CHAT_TOPIC_PREFIX + roomId, chatMessageResponse); | |
| } catch (Exception e) { | |
| log.error("Failed to send message to WebSocket topic: {}", e.getMessage()); | |
| // TODO: 추가 보상 트랜잭션 처리 또는 재시도 로직 구현 | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/chat/service/ChatService.java
around lines 139 to 155, improve the sendChatMessage method by defining the
"/topic/chat/" string as a constant to avoid hardcoding, add error handling
around the simpMessagingTemplate.convertAndSend call to manage messaging
failures gracefully, and consider adjusting the transaction scope or using
compensating actions to ensure atomicity between saving the chat message to the
database and sending the message over the messaging template.
src/test/java/com/example/solidconnection/chat/fixture/ChatAttachmentFixtureBuilder.java
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/chat/fixture/ChatAttachmentFixtureBuilder.java
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/chat/fixture/ChatRoomFixtureBuilder.java
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/chat/repository/ChatReadStatusRepositoryForTest.java
Show resolved
Hide resolved
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
고생하셨습니다! 순환참조 문제까지 발생해서 쉽지 않으셨겠네요... 제 생각에 시큐리티 관련 문제는 추후에 리팩토링하는 게 좋을 거 같습니다! 궁금한 점들만 남겨놓았습니다~
| public record SiteUserPrincipal(Long id, String email) implements Principal { | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return this.email; | ||
| } | ||
| } |
There was a problem hiding this comment.
이건 사용하고있지 않은 거 같은데 왜 생성된건지 알 수 있을까요?
There was a problem hiding this comment.
이것저것 해 보다가 포함된 것 같습니다. 제거했고, 없어도 잘 동작하는 거 확인했습니다 !
| Object userAttribute = attributes.get("user"); | ||
|
|
||
| if (userAttribute instanceof Principal) { | ||
| Principal principal = (Principal) userAttribute; | ||
| return principal; | ||
| } |
There was a problem hiding this comment.
attributes.get("user")를 그대로 리턴하는 게 어떤 의미인지 알려주실 수 있나요!?
지금 보기에는 super.determineUser()와 똑같아보여서요..!
There was a problem hiding this comment.
<3줄 요약>
- 두 로직은 핸드셰이크가 수행된 쓰레드와 WebSocket 관련 쓰레드가 같은 경우 같은 동작을 수행합니다.
- 그럼에도
attributes.get("user")를 사용하여Principal을 리턴한 이유는,Principal객체를 안전하게 전달하기 위해attributes에 담았고, 그걸 꺼내 리턴한 것입니다. super.determineUser()는 혹시나 꺼낸 값이Principal이 아닌 경우 예외 터뜨리지 않고 기본 동작을 수행하도록 하기 위해 작성하였습니다.
저 코드 구현 당시 제 생각은 ...
Principal 객체를 어떻게 WebSocket 세션에 전달할지 찾아보았고, 커스텀 핸들러에서 determineUser 메서드를 구현하여 전달한다는 것을 알게 되었습니다.
determineUser 메서드에 대해 잘 몰라서 정의를 살펴보았고, attribute 파라미터가 WebSocket 세션에 전달하고 싶은 데이터를 저장하는 임시 저장소 역할을 한다는 것을 알게 되었습니다.
@Override
public boolean beforeHandshake(ServerHttpRequest request, ServerHttpResponse response,
WebSocketHandler wsHandler, Map<String, Object> attributes) {
Principal principal = request.getPrincipal();
if (principal != null) {
attributes.put("user", principal);
return true;
}
return false;
}그래서 인터셉터에서 핸드셰이크 전 Principal 을 user 라는 키로 attributes 에 저장하였고, 핸들러에서 user 에 해당하는 값을 꺼내 간단히 Principal 인지 검증 후 리턴하도록 구현하였습니다.
혹시 만약에 꺼낸 값이 Principal 이 아닌 경우 관련 처리도 필요했고, 그 경우 부모 클래스의 기본 동작을 따르도록 하였습니다.
제가 첨부한 블로그는 어쨌거나 attributes 인자를 사용하지 않았고, 잘 동작한 것 같습니다. 그럼 attributes 는 사용 안 해도 되는 거 아닐까 ? 해서 찾아보았는데, WebSocket 핸드셰이크를 수행하는 쓰레드와 실제 WebSocket을 사용하여 메시지를 주고받는 쓰레드가 다를 수 있으며, 이 경우 SecurityContextHolder 로부터 Principal 을 가져올 수 없습니다. 생성된 Principal 은 같은 쓰레드 내에서만 유효합니다. [참고]
| String[] parts = destination.split("/"); | ||
| if (parts.length < 3 || !parts[1].equals("topic")) { | ||
| if (parts.length < 4 || !parts[1].equals("topic") || !parts[2].equals("chat")) { | ||
| throw new CustomException(ErrorCode.INVALID_ROOM_ID); | ||
| } | ||
| return parts[2]; | ||
| return parts[3]; |
There was a problem hiding this comment.
여긴 정규표현식을 좀 활용하면 어떤가요?
private static final Pattern ROOM_ID_PATTERN = Pattern.compile("^/topic/chat/(\\d+)$");
src/main/java/com/example/solidconnection/chat/dto/ChatMessageSendRequest.java
Show resolved
Hide resolved
| private final ChatReadStatusRepository chatReadStatusRepository; | ||
| private final SiteUserRepository siteUserRepository; | ||
|
|
||
| private final SimpMessagingTemplate simpMessagingTemplate; |
There was a problem hiding this comment.
convertAndSend라는 기능정도만 사용하고 있는데 추상화된 SimpMessageSendingOperations가 아니라 구체화된 클래스인 SimpMessagingTemplate를 사용하신 이유가 있으신가요?
There was a problem hiding this comment.
제가 이전에 WebSocket 건드려봤을 때 SimpMessagingTemplate 만 사용했고, SimpMessageSendingOperations의 존재는 몰랐습니다 .. 인터페이스를 의존해야 하는 게 맞기에, 수정했습니다 !
| @MessageMapping("/chat/{roomId}") | ||
| public void sendChatMessage( | ||
| @DestinationVariable Long roomId, | ||
| @Payload ChatMessageSendRequest chatMessageSendRequest, |
There was a problem hiding this comment.
오 전 @RequestBody를 사용해도 되는줄 알았는데 웹소켓에선 @payload가 표준인가 보네요!
|
|
||
| ChatMessage chatMessage = new ChatMessage( | ||
| chatMessageSendRequest.content(), | ||
| siteUserId, |
There was a problem hiding this comment.
senderId를 siteUserId가 아니라 ChatParticipant의 id로 하자고 이야기했던 거 같긴한데 혹시 다른 논의가 있었나요!?
There was a problem hiding this comment.
맞습니다 ... 구현에 급급해서 놓쳤습니다 ㅠㅠ 찾아주셔서 감사합니다 !!!
어차피 ChatParticipant의 id를 가져와야 하므로 validateChatRoomParticipant 는 제거하였습니다 !
- topic/{roomId} -> topic/chat/{roomId}
- 의미적 명확성을 위해
- 이에 컨트롤러 인자로 siteUserId를 받도록 하고, DTO에 senderId를 삭제한다.
- 검증 로직이 핸들러에서 사용됨에 따라 발생하는 순환 참조를 막기 위해 Lazy 어노테이션을 사용한 생성자를 직접 작성
- 정규표현식을 사용하여 채팅방 ID 추출 - DTO 검증 추가 - 구체화 클래스가 아닌 인터페이스 사용하도록 (DIP) - senderId가 siteUserId가 아니라 chatParticipantId로 설정되도록 변경
whqtker
force-pushed
the
feat/421-chat-with-websocket
branch
from
86a2c05 to
338780f
Compare
2 participants
관련 이슈
참고 사항
작업 내용
WebSocket과 STOMP 프로토콜을 사용하여 기본 텍스트 채팅(첨부파일 X) 송수신을 구현하였습니다.
생각보다 동작이 매우 복잡하기에, 저도 좀 정리할 겸 전체적인 흐름을 매우x5 자세히 보여드리며 구현 내용을 설명드리겠습니다.
init
StompWebSocketConfig->StompHandler->ChatService->SimpMessagingTemplate->StompWebSocketConfig형태의 순환 참조가 존재합니다. 이에SimpMessagingTemplate에 대한 프록시 객체를 생성합니다. 왜 발생하게 되었는지는 추후 서술하겠습니다. 4748087StompWebSocketConfig의registerStompEndpoints메서드에 의해/connect엔드포인트에 대한 인터셉터와 핸들러를 등록합니다. 인터셉터와 핸들러를 추가로 구현하여 등록하게 된 이유는 추후 서술하겠습니다.configureClientInboundChannel메서드에 의해 인바운드 채널에 핸들러와 커스텀 풀 설정을 적용합니다.configureMessageBroker메서드에 의해 브로커를 설정합니다.WebSocket Handshake
Authorization헤더와 JWT 토큰과 함께ws://localhost:8080/connect로 HTTP GET 핸드셰이크 요청을 보냅니다.TokenAuthenticationFilter에서 토큰 검증 후SiteUserDetails을 담은TokenAuthentication객체가SecurityContextHolder에 저장됩니다.beforeHandshake메서드로 전달됩니다. 인터셉터를 구현한 이유는 핸드셰이크를 통해 인증된 사용자 정보Principal을 WebSocket 세션에 전달해야 이후 STOMP를 통한 메시지 송수신 시 매번 JWT와 같은 인증 정보를 포함하지 않아도 되기 때문입니다.Principal을 세션에 저장하는 동작을beforeHandshake메서드가 수행합니다. 0d02171determineUser메서드가 WebSocket 세션의 실제Principal객체를 리턴합니다.STOMP 세션 수립 (Connect)
CONNECT프레임을 전송하여 STOMP 세션을 수립해야 합니다. 성공적으로 수립되면 서버는CONNECTED프레임을 클라이언트에 전송합니다.clientInboundChannel로 전달되면, STOMP 핸들러가 이를 가로채preSend메서드를 수행합니다.preSend메서드는 프레임이CONNECT,SUBSCRIBE인 경우 적절한 검증을 수행합니다. (참고:SEND프레임을 전송하는 주체는 이미CONNECT에 대한 검증을 거쳤으므로 신뢰합니다.)accessor.getUser()를 통해Principal객체를 가져옵니다.determineUser메서드로 인해 가능한 동작입니다 !CONNECTED프레임을 전송하고 마무리합니다.Subscribe
SUBSCRIBE프레임을 전송하고, 디코더가 이를 디코딩합니다.clientInboundChannel로 메시지를 전달하고, 핸들러가 이를 가로채서 마찬가지로Principal객체를 가져와 검증합니다.ChatService에 구현된 검증 메서드를 사용합니다. (여기서 순환 참조가 발생했고,@Lazy어노테이션을 사용해서 생성자를 직접 구현할 수밖에 없었습니다 ㅠㅠ)Send
SEND프레임 전송하고 디코딩합니다. (한글이라 인코딩이 깨졌네요) 핸들러가 메시지를 가로채나,SEND이므로 그냥 통과합니다./publish를 보고@MessageMapping으로 라우팅합니다.roomId를 추출하고, 메시지 JSON을 DTO로 변환하고, 해당 세션의Principal을 가져옵니다. 이 또한determineUser메서드로 인해 가능한 동작입니다. 9013247siteUserId를 추출하고, 비즈니스 로직을 수행합니다. 정상적으로 메시지가 저장되었습니다.MESSAGE프레임을 전송합니다. 정상적으로 메시지 수신되었습니다.특이 사항
/topic/{roomId}에서 메시지 수신하도록 구현되었는데, 채팅 관련 토픽임을 강조할 필요가 있다고 생각하여/topic/chat/{roomId}로 변경하였습니다. 추후 STOMP 기반으로 동작하는 다른 기능 추가 시 확장성이 더 좋을 것 같다는 판단이었습니다. 8d0e914리뷰 요구사항 (선택)
create_at)으로 설정할까 ? 하다 일단 배제하고 구현했습니다. 현재는 서버에 도착한 시간을 기준으로create_at을 설정하고 있는데, 만약 서버 부하 등으로 인해 채팅 메시지가 서버에 늦게 도착하는 경우 정합성 문제가 있을 것 같습니다.