-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feat #25 채팅 비즈니스 로직 및 예외처리 구현 #25
The head ref may contain hidden characters: "feat/#18/\uCC44\uD305-\uBE44\uC988\uB2C8\uC2A4-\uB85C\uC9C1"
Conversation
@RequiredArgsConstructor | ||
public class ChattingRoomService { | ||
|
||
private static final String ENTER_MESSAGE =" 님이 입장하셨습니다."; |
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.
환경변수 처리하는게 나을까요?
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.
입장 메세지는 잘 안바뀔 거 같습니다!
상수 처리해도 괜찮을 거 같아요!
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.
저도 현재로서는 상수로 처리하는 게 간결하고 유지보수에도 적합해보여요
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.
고생하셨습니다 !!
} | ||
|
||
@Transactional | ||
public void subscribeChatRoom(long roomId, long senderId, String senderName) { |
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.
subscribeChatRoom에서 현재 멤버가 이미 구독 중인 방인지 중복 체크 로직이 구현되어 있을까요 ?? 동일 사용자가 동일한 방에 여러 번 구독하면 메시지 수신 시 중복 처리가 발생할 수 있다고 생각해서, 같은 방을 여러 번 구독하지 않도록 제한하는 로직이 없다면 서비스단에서 해당 로직이 필요해보입니다
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.
생각을 못햇네요 추가하겠습니다!
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이 커질 것 같아서
다음 작업에서 예외 처리 및 핸들러 구현 진행 하겠습니다!
@Value("${chat.topic}") | ||
public String chatTopic; | ||
|
||
public void chat(ChatMessageRequest request, SimpMessageHeaderAccessor accessor) { |
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.
해당 chat 메서드에서 Redis를 통해 메세지 발행 -> MongoDB 에 저장하는 구조로 이해했는데,
저장과 발행이 동시에 이루어지게되면 엣지 케이스로 Redis 발행이 실패할 경우에 메시지는 Redis 단에서 발행되지 않지만
DB에는 저장되는 문제가 발생할 수도 있어보입니다
이런 경우 클라이언트는 해당 메시지를 수신하지 못하고, DB상에는 메시지가 남아있는 예외가 발생할 수도 있을거같아서
해당 chat 메세지 단에서, 혹은 다른 메서드를 이용해서 Redis 발행 성공 여부를 확인한 후에 성공 시에만 데이터를 저장하는 로직을 추가해주면 안정성 측면에서도 좋아보이는데 어떻게 생각하실까요 ??
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.
실패 여부를 확인할 수 있는 방법을 찾아 보겠습니다
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.
생각을 해봤는데, Redis의 전달 실패 여부와 상관 없이 DB에는 저장을 하는게 좋을 것 같아요! 사용자가 채팅방에 나갔다 들어오면 다시 메시지를 볼 수 있는게 무결성을 지킬 수 있는 방법이 될 수 있을 것 같아요! 어떻게 생각하시나요!
그와는 별개로 Redis가 전송을 실패하는 경우를 직접적으로 체크할 수 있는 방법이 있는 것은 아니지만 차후 작업에서 재시도 전략을 추가하는게 서비스의 신뢰도에 좋을 것 같아요!
해당 부분도 차후 작업에서 함께 진행하도록 하겠습니다!
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.
pub/sub이 비동기식 전송이라 실패여부를 판단하기 어렵나 보군요..!!
알겠습니다!
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.
고생하셨습니다!
|
||
@Override | ||
public void registerStompEndpoints(StompEndpointRegistry registry) { | ||
registry | ||
.setErrorHandler(stompExceptionHandler) | ||
.addEndpoint("/ws") | ||
.setAllowedOriginPatterns("http://localhost:3001"); |
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.
포트 번호를 3001로 지정한 이유가 궁금합니다!
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.
제가 3001로 사용 중인데 저번 피알에서 그대로 올려버려서.. 수정 하겠습니다
@GetMapping("/api/chat/room") | ||
public ApiResponse<ChattingRoomResponse> createChattingRoom() { | ||
ChattingRoomResponse response = chattingRoomService.save(); | ||
|
||
return ApiResponse.response(OK, CREATE_CHATTING_ROOM_SUCCESS.getMessage(), response); | ||
} |
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.
채팅방을 생성하는 메서드로 이해했습니다.
그렇다면 해당 API는 DB에 상태를 변화 (데이터 생성)을 야기할 거 같습니다.
그럼에도 @GetMapping을 사용한 이유가 따로 있을까용??
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.
음 채팅방 생성 자체는 매칭방 생성 이벤트 발생시 내부적으로 이루어질 것 같아서 임시로 구현을 해뒀습니다!
생성 보다는 채팅방 정보 조회에 집중해서 GET을 썼던 것 같은데 흠 고민이네용..
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.
생성이니까 Post로 수정 했습니다!
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.
고생하셨습니다!
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.
고생하셨습니당👍
// ChattingParticipant chattingParticipant = ChattingParticipant.of(chattingRoom, members); | ||
// chattingParticipantRepository.save(chattingParticipant); |
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.
주서으로 처리된 코드는 필요없는 코드인가요??
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.
멤버 서비스가 다른분 작업에서 진행이 된 것으로 알고 있어서 머지 후에 다음 작업에서 이어서 작업하려고 했습니당
제가 작업할 땐 아직 머지가 안된 상태여서..
📌 관련 이슈
관련 이슈 번호 #18
Close #
🚀 작업 내용
채팅을 위한 비즈니스 로직을 작성 했습니다
웹소켓 접속, 채팅방 생성, 채팅방 구독, 채팅 메시지 보내기 구현 완료 했습니다
userId는 웹소켓 접속(CONNECT)시 세션에 저장, roomId는 채팅방 구독(SUBSCRIBE)시 동일한 세션에 저장하여 메시지를 보낼 때 꺼내와서 사용하도록 구현했습니다.
그 이유는 userId는 인증이 중요하다고 생각했고, roomId는 클라이언트 단에서 잘못 입력되면 다른 채팅방에 메시지가 전달되는 것을 막기 위해서 입니다.
STOMP 인터셉터에서 발생하는 예외처리 구현 했습니다.
구독 경로가 올바르지 않을 때, 메시지 경로가 올바르지 않을 때, 연결시 jwt 토큰이 올바르지 않을 때 예외가 발생하고, 웹소켓 연결이 끊어지며 예외 메시지가 STOMP로 전달됩니다.
비즈니스 로직 예외 핸들러와 세부 로직은 다음 작업에서 진행할 계획입니다
📸 스크린샷
인터셉터에서 예외 발생시
채팅화면
DB
📢 리뷰 요구사항