-
Notifications
You must be signed in to change notification settings - Fork 2
Feat(be) : SSE 설정 삭제 및 Controller 분리 및 WebSocket 설정 #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/domain/userChat/UserChatSseEvents.kt
🟢 좋은점:
- 이 파일이 WebSocket/STOMP 도입 전 임시 SSE 구현으로 명시되어 있으며, 테스트 후 제거 예정으로 주석 처리된 점이 적절합니다. 불필요한 임시 코드를 실제로 삭제함으로써 코드베이스를 정리하고 유지보수성을 높였습니다. 이는 Kotlin 최적화 측면에서 불필요한 컴포넌트를 제거한 좋은 결정입니다.
- ktlint 규칙 준수: 코드가 간결하고 네이밍 컨벤션(예:
subscribe,publishNew)이 일관되며, 포맷팅이 깨끗합니다. 삭제로 인해 더 이상 유지할 필요가 없어집니다.
🟡 개선사항:
- 삭제 전에 SSE 구현의 핵심 로직(예:
ConcurrentHashMap을 사용한 emitters 관리)을 WebSocket/STOMP 전환 시 재사용 가능한 유틸리티나 테스트 케이스로 추출하는 것을 고려할 수 있습니다. 만약 유사한 패턴이 반복될 가능성이 있다면, 이를 문서화하거나 별도 모듈로 보관하는 게 도움이 될 수 있습니다. - Kotlin 최적화 관점에서, 기존 코드의
emitter.onCompletion과onTimeout핸들링은 좋았으나, 더 안전한 null safety를 위해let이나also확장 함수를 활용할 수 있었을 텐데, 삭제로 인해 이제 해당되지 않습니다.
🔴 문제점:
- 없음. 이 파일은 임시 구현으로 설계되었고, 삭제가 의도된 대로 진행되었으므로 반드시 수정할 부분이 없습니다. 다만, WebSocket/STOMP 전환 후 SSE 관련 테스트가 완전히 제거되었는지 확인하세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/domain/userChat/chatmessage/controller/ChatMessageController.kt
🟢 좋은점:
- ApiResponse를 모든 API 응답에 일관되게 사용하고 있으며, 성공 메시지(msg)를 포함해 표준화된 응답 형식을 따름. WebSocket 메시징에도 동일한 ApiResponse를 적용해 일관성 유지.
- Kotlin의 null safety를 잘 활용: @RequestParam의 after를 Long?으로 처리하고, 조건부 로직(if-else)에서 null 체크.
- RESTful 엔드포인트 설계가 적절: GET으로 메시지 목록 조회, POST로 메시지 전송, PathVariable과 RequestParam을 적절히 사용.
- SimpMessagingTemplate을 통해 실시간 WebSocket 브로드캐스팅 구현, 채팅 기능에 적합.
🟡 개선사항:
- Kotlin 최적화: if-else 로직을 when 표현식으로 변경하면 더 idiomatic하게 작성 가능 (e.g.,
val messages = when { after == null -> msgSvc.getlistbefore(roomId, limit); else -> msgSvc.getlistafter(roomId, after) }). - 네이밍 컨벤션(ktlint): 생성자 파라미터
msgSvc를chatMessageService처럼 camelCase로 변경. Service 메서드명getlistbefore,getlistafter를getListBefore,getListAfter로 수정해 일관성 높임. - ApiResponse의 제네릭 타입:
ApiResponse<Any>대신 구체적 타입 (e.g.,ApiResponse<List<ChatMessage>>또는ApiResponse<ChatMessage>)으로 명시하면 타입 안전성 향상. Any는 런타임 오류 가능성 증가. - ktlint 포맷팅: 긴 문자열 할당 (e.g.,
ApiResponse(msg = "메시지 조회", data = messages))을 여러 줄로 나누거나, 상수로 추출해 가독성 개선. limit 파라미터에 상한/하한 검증 (e.g., 1~100) 추가 고려. - 글로벌 익셉션 처리: @ControllerAdvice가 글로벌로 적용된다고 가정하나, 이 컨트롤러에서 발생할 수 있는 예외 (e.g., roomId 유효성, after 값 범위)를 try-catch로 로컬 처리하거나, 표준 에러 응답 (
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/domain/userChat/chatmessage/controller/ChatMessageSocketController.kt
🟢 좋은점:
- ApiResponse를 WebSocket 메시지 전송 시 일관되게 사용하고 있으며, 성공 응답 형식(메시지와 데이터 포함)이 표준화되어 있습니다. 이는 모든 API 응답 규칙을 잘 따르고 있습니다.
- Kotlin 최적화 측면에서 의존성 주입(constructor injection)이 적절히 사용되었고, 코드가 간결하며 null safety가 기본적으로 준수된 구조입니다. (예: Long 타입의 roomId와 req 객체 처리)
- ktlint 규칙 준수: 포맷팅(들여쓰기, 줄바꿈)이 깔끔하고, 네이밍 컨벤션(클래스명, 메서드명, 변수명)이 Kotlin/Spring 표준을 따르고 있습니다. 패키지 구조도 논리적입니다.
🟡 개선사항:
- Kotlin 최적화: SendMessageReq가 data class로 가정되지만, 명시적으로 확인되지 않았습니다. 만약 data class가 아니라면 data class로 변환하여 불변성과 toString() 등의 유틸리티를 활용하는 것이 좋습니다. 또한, when 표현식이나 확장 함수를 활용해 roomId 유효성 검사(예: isValidRoomId 확장 함수)를 추가하면 코드가 더 표현력 있게 될 수 있습니다.
- ktlint 규칙: 전체적으로 준수되지만, 긴 줄(예: convertAndSend 호출)이 있으므로 100자 이내로 줄이는 포맷팅을 고려하세요. 네이밍에서 "handleMessage" 메서드명을 "sendChatMessage"처럼 더 구체적으로 변경하면 의도를 명확히 할 수 있습니다.
- 글로벌 익셉션 처리: WebSocket 컨트롤러이므로 HTTP @ControllerAdvice가 직접 적용되지 않을 수 있습니다. @MessageExceptionHandler를 추가하거나, 서비스 레이어에서 예외를 ApiResponse로 래핑하는 확장 함수를 도입해 일관된 에러 응답을 보장하세요.
🔴 문제점:
- 글로벌 익셉션 처리: 현재 컨트롤러에 try-catch나 예외 핸들링이 전혀 없어, send() 메서드에서 발생하는 예외(예: InvalidRoomException)가 그대로 브로드캐스트되거나 무시될 위험이
JIWONKIMS
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다음 PR때 서비스 중에서 단순 조회인 메서드는 readonly 설정해주시면 감사하곘습니다
-1단계 HTTP 만 사용한 통신 완료 후 WebSocket,STOMP 전환을 위한 SSE 삭제
-room,message 컨트롤러 분리
-WebSocket 설정
DTO,인증,예외처리 진행 예정