test: WebSocket 관련 테스트 코드 작성 #430
Conversation
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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: 3
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/chat/config/CustomHandshakeHandler.java (1)
14-26: WebSocket 세션 Principal 할당 로직이 안전하게 구현되었습니다!구현의 장점들:
- 안전한 타입 체크:
instanceof로 Principal 타입 확인 후 캐스팅- 적절한 폴백: 기본 동작으로 안전하게 처리
- 명확한 역할: 핸드셰이크 인터셉터에서 설정한 "user" 속성 활용
아래와 같이 코드를 약간 간소화할 수 있습니다:
Object userAttribute = attributes.get("user"); -if (userAttribute instanceof Principal) { - Principal principal = (Principal) userAttribute; - return principal; -} +if (userAttribute instanceof Principal principal) { + return principal; +} return super.determineUser(request, wsHandler, attributes);src/main/java/com/example/solidconnection/chat/config/WebSocketHandshakeInterceptor.java (1)
20-25: 로깅 추가로 디버깅을 개선해보세요.핸드셰이크 실패 시 로깅을 추가하면 디버깅에 도움이 될 것 같습니다.
if (principal != null) { attributes.put("user", principal); return true; } + log.debug("WebSocket handshake failed: no authenticated principal found"); return false;src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java (1)
67-113: 추가 테스트 케이스 고려사항현재 테스트는 핸드셰이크만 검증하고 있습니다. STOMP 구독과 메시지 전송에 대한 통합 테스트도 추가하면 더 완전한 커버리지를 얻을 수 있을 것 같습니다.
STOMP 구독 및 메시지 전송 테스트 코드를 생성해드릴까요?
src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java (1)
376-423: 테스트 커버리지 추가 고려사항현재 테스트는 기본적인 성공/실패 케이스를 다루고 있습니다. 다음과 같은 추가 테스트 케이스를 고려해보시기 바랍니다:
- 빈 메시지 내용 전송 시 검증
- 존재하지 않는 채팅방에 메시지 전송
- 메시지 길이 제한 검증
추가 테스트 케이스 구현을 도와드릴까요?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/example/solidconnection/chat/config/CustomHandshakeHandler.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/ChatMessageController.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/service/ChatService.java(5 hunks)src/main/java/com/example/solidconnection/security/config/SecurityConfiguration.java(2 hunks)src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java(4 hunks)src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java
🔇 Additional comments (20)
src/main/java/com/example/solidconnection/security/config/SecurityConfiguration.java (2)
9-9: 임포트 순서 정리 완료!코드 정리가 잘 되었습니다.
65-65: WebSocket 엔드포인트 보안 설정 추가가 적절합니다
/connect/**경로에 대한 인증 요구사항 추가가 올바르게 구현되었습니다. 다음과 같은 장점들이 있습니다:
- 보안 강화: WebSocket 연결 시 인증된 사용자만 접근 가능
- 올바른 순서: admin 규칙보다 먼저 배치되어 적절한 보안 필터 체인 구성
- 통합성: 새로 추가된 WebSocket 핸드셰이크 인터셉터와 완벽하게 연동
src/main/java/com/example/solidconnection/chat/dto/ChatMessageSendRequest.java (1)
6-12: 채팅 메시지 요청 DTO 구현이 깔끔합니다!다음과 같은 좋은 구현 특징들이 있습니다:
- 적절한 유효성 검증:
@NotNull로 필수값 검증@Size(max = 500)로 메시지 길이 제한- 명확한 에러 메시지: 사용자 친화적인 한국어 메시지 제공
- 모던 자바 활용: Record 클래스로 간결한 구조
채팅 메시지 전송 기능의 견고한 기반이 될 것 같습니다.
src/main/java/com/example/solidconnection/chat/dto/ChatMessageSendResponse.java (1)
5-19: 채팅 메시지 응답 DTO 설계가 우수합니다!구현의 장점들을 정리하면:
- 적절한 필드 선택:
messageId: 메시지 식별을 위한 필수 정보content: 메시지 내용senderId: 발신자 식별 정보- 팩토리 메서드 패턴:
from()메서드로 도메인 객체에서 깔끔한 변환- 캡슐화: 도메인 객체의 내부 구조를 숨기고 필요한 정보만 노출
WebSocket을 통한 실시간 메시지 브로드캐스트에 최적화된 구조입니다.
src/main/java/com/example/solidconnection/chat/config/StompWebSocketConfig.java (2)
25-26: WebSocket 인증 컴포넌트 의존성 주입이 적절합니다새로운 인증 관련 컴포넌트들이 올바르게 추가되었습니다:
WebSocketHandshakeInterceptor: 핸드셰이크 시 인증 정보 추출CustomHandshakeHandler: WebSocket 세션의 Principal 할당
32-36: STOMP 엔드포인트 설정이 보안과 호환성을 모두 고려하여 완벽합니다!엔드포인트 구성의 우수한 점들:
보안 강화:
addInterceptors(webSocketHandshakeInterceptor): 핸드셰이크 시 인증 정보 검증setHandshakeHandler(customHandshakeHandler): Principal 할당으로 세션 인증 상태 관리호환성 보장:
.withSockJS(): 다양한 브라우저와 네트워크 환경 지원 (이전 리뷰 학습 내용 반영 완료)CORS 설정: 허용된 오리진 적절히 구성
인증된 사용자만 WebSocket에 연결할 수 있는 견고한 구조가 완성되었습니다.
src/main/java/com/example/solidconnection/chat/controller/ChatMessageController.java (1)
21-31: 메시지 전송 로직이 깔끔하게 구현되었습니다.WebSocket 엔드포인트 매핑과 검증 로직이 적절히 구성되어 있고, 채팅 서비스와의 연동도 명확합니다.
src/main/java/com/example/solidconnection/chat/config/WebSocketHandshakeInterceptor.java (1)
15-26: 핸드셰이크 인터셉터 로직이 올바르게 구현되었습니다.
- 인증된 Principal 추출 및 저장
- 인증되지 않은 사용자의 연결 차단
이 구조가 WebSocket 보안을 적절히 강화하고 있습니다.
src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java (3)
79-96: 인증된 사용자 테스트가 체계적으로 구성되었습니다.
- JWT 토큰 생성
- Authorization 헤더 설정
- 핸드셰이크 성공 검증
- 전송 에러 부재 확인
테스트 로직이 명확하고 검증 항목이 적절합니다.
98-112: 인증 실패 테스트가 정확히 구현되었습니다.
- Authorization 헤더 없이 연결 시도
- HTTP 401 Unauthorized 예외 발생 확인
- 전송 에러 발생 검증
예외 처리와 에러 상황 테스트가 잘 구성되어 있습니다.
55-57: SockJS 설정 적용 확인 – 리뷰 지적 무시 요망
- 확인된 설정
- StompWebSocketConfig.java의
registry.addEndpoint("/connect")호출 체인에 이미.withSockJS()가 포함되어 있습니다.- 결론
- SockJS 구성은 이미 적용되어 있어 추가 설정이 필요 없습니다.
Likely an incorrect or invalid review comment.
src/main/java/com/example/solidconnection/chat/config/StompHandler.java (2)
33-36: Principal 인증 검증 로직이 일관성 있게 적용되었습니다.CONNECT와 SUBSCRIBE 모두에서 Principal 존재 여부를 확인하는 것이 보안상 적절합니다.
Also applies to: 40-43
25-25: 정규표현식을 사용한 roomId 추출이 깔끔합니다.이전 split 방식보다 정규표현식을 사용한 패턴 매칭이 더 안전하고 명확한 검증을 제공합니다.
Also applies to: 62-67
src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java (3)
71-72: SimpMessagingTemplate 모킹 설정이 적절합니다.WebSocket 메시지 전송을 테스트하기 위한 MockBean 추가가 올바르게 구성되었습니다.
376-410: 채팅방 참여자 메시지 전송 테스트가 체계적으로 구성되었습니다.
- 메시지 전송 요청 생성
- chatService.sendChatMessage 호출
- ArgumentCaptor로 destination과 payload 검증
- 메시지 내용과 발신자 ID 확인
테스트 구조가 명확하고 검증 항목이 완전합니다.
412-422: 비참여자 메시지 전송 제한 테스트가 정확합니다.채팅방 참여자가 아닌 사용자의 메시지 전송 시도 시 CHAT_PARTICIPANT_NOT_FOUND 예외가 발생하는 것을 올바르게 검증하고 있습니다.
src/main/java/com/example/solidconnection/chat/service/ChatService.java (4)
13-14: 새로운 DTO 임포트 추가 승인채팅 메시지 송신 기능을 위한
ChatMessageSendRequest와ChatMessageSendResponseDTO 임포트가 적절하게 추가되었습니다.
29-29: WebSocket 메시징을 위한 의존성 주입 적절함
@Lazy애노테이션과SimpMessageSendingOperations의존성 추가가 올바르게 구현되었습니다- 순환 의존성 방지를 위한
@Lazy사용이 적절합니다Also applies to: 32-32, 45-45
110-115: 메서드 가시성 변경 승인
validateChatRoomParticipant메서드가private에서public으로 변경되었습니다. 이는StompHandler에서 채팅방 구독 권한 검증을 위해 필요한 적절한 변경사항입니다.
47-59: 검증 완료: 명시적 생성자 구현이 올바르게 적용되었습니다.
- Lombok 어노테이션 제거
- 클래스 상단에 @requiredargsconstructor 어노테이션이 더 이상 없습니다.
- 필드 초기화 확인
- 선언된 모든 final 필드가 생성자 내부에서 정확히 초기화됩니다.
변경사항이 의도대로 반영되었으므로 추가 검토는 필요하지 않습니다. 😊
src/main/java/com/example/solidconnection/chat/controller/ChatMessageController.java
Show resolved
Hide resolved
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
저도 ArgumentCaptor는 처음보는데 sendChatMessage를 테스트하기가 쉽지 않았네요.. 간단한 리뷰만 남겨놓아서 Approve해놓겠습니다!
| ArgumentCaptor<String> destinationCaptor = ArgumentCaptor.forClass(String.class); | ||
| ArgumentCaptor<ChatMessageSendResponse> payloadCaptor = ArgumentCaptor.forClass(ChatMessageSendResponse.class); | ||
|
|
||
| Mockito.verify(simpMessagingTemplate).convertAndSend(destinationCaptor.capture(), payloadCaptor.capture()); |
There was a problem hiding this comment.
Mockito.verify(simpMessagingTemplate).convertAndSend(destinationCaptor.capture(), payloadCaptor.capture());
이부분만 import static org.mockito.BDDMockito.verify;
Mocktio가 아닌 BDDMocito로 변경해주실 수 있나요?
There was a problem hiding this comment.
변경했습니다 ! Mockito 대신 BDDMockito 를 import하도록 변경했습니다
eae5e66
whqtker
force-pushed
the
test/427-websocket-test
branch
from
eae5e66 to
5a2ad88
Compare
2 participants
관련 이슈
작업 내용
WebSocket + STOMP 관련 테스트 코드와 메시지 송신 테스트를 작성하였습니다.
#423 이 머지되면 rebase 하고 머지하겠습니다 !
특이 사항
ArgumentCaptor라는 걸 처음 써 봤는데요, 어색한 부분이 있다면 말씀해주세요 ~WebSocket + STOMP 통합 테스트는
chat도메인 하위가 아니라 따로 작성하였습니다.리뷰 요구사항 (선택)
추가할 테스트가 있는지