Ping 통신을 위한 데이터 타입 변경 #773
Conversation
| if (ChattingType.PING == type) { | ||
| return createPingResponse(sessionAttribute, data, session); | ||
| } | ||
| return createSendMessageResponse(data, sessionAttribute); |
There was a problem hiding this comment.
선택
당장 진행하지 않아도 괜찮긴 한데, WebSocketHandler에서 provider를 통해 chat 혹은 bid로 이동하는 것과 같이, chat type에 따라 클래스를 분리해 핸들링하는 것에 대해 어떻게 생각하시나요?
현재 해당 클래스에서 많은 역할이 있는 것 같아 의견 여쭤봅니다!
There was a problem hiding this comment.
좋은 의견 감사합니다!
말씀해주신 것처럼 WebSocketHandler에서 너무 많은 역할을 하고 있어서 chat type에 따라 클래스를 분리해 핸들링하도록 수정했습니다.
추가로 WebSocketHandler에서 PingHandler와 MessageHandler를 호출하여 데이터를 넘겨줄 때
WebSocketSession이나 Map<String, String>과 같은 원시 타입이 아닌 dto로 포장한 값을 전달하도록 수정했습니다.
원시 타입을 넘겨주니 원시 타입 내부에 어떤 데이터를 포함하고 있는지 모르겠더라구요!
이 부분이 괜찮은지 제이미의 의견이 있다면 궁금합니다!
| private List<SendMessageDto> createPingResponse(final SessionAttributeDto sessionAttribute, final Map<String, String> data, final WebSocketSession userSession) throws JsonProcessingException { | ||
| final ChatPingDataDto pingData = objectMapper.convertValue(data, ChatPingDataDto.class); | ||
| final ReadMessageRequest readMessageRequest = new ReadMessageRequest(sessionAttribute.userId(), pingData.chatRoomId(), pingData.lastMessageId()); | ||
| final List<ReadMessageDto> readMessageDtos = messageService.readAllByLastMessageId(readMessageRequest); |
There was a problem hiding this comment.
필수
해당 클래스에 기존 컨벤션이었던 120자를 초과하는 라인들이 많이 보이네요!!
그리고 메서드 순서도 호출순으로 보이지 않습니다..!
There was a problem hiding this comment.
인텔리제이 설정이 초기화되어서 정렬이 잘 안 되고 있었네요...ㅠㅠ
감사합니다. 수정했습니다!
| MESSAGE("message"), | ||
| PING("ping"), | ||
| ; |
There was a problem hiding this comment.
질문
대소문자를 무관하게 하는 로직으로 수정하는 과정에서 value는 매핑을 위해 있는 것으로 보여 해당 값들을 없애고 name과 비교하는 로직으로 수정하려고 합니다. 괜찮을까요?
괜찮으시다면 제 쪽에서 추후 수정하려고 하는데, 메리의 코드기에 의견을 구하고 싶어 질문드립니다!
|
|
||
| // when | ||
| assertThatThrownBy(() -> provider.handleCreateSendMessage(writerSession, 잘못된_메시지_전송_데이터)) | ||
| .isInstanceOf(IllegalArgumentException.class); |
There was a problem hiding this comment.
메시지 내부 필드를 비교해야 하는지에 대한 질문이 맞나요?!
📄 작업 내용 요약
Ping 통신을 위한 데이터 타입을 변경했습니다.
data에 해당하는 값에 ping, message를 구분하는
type을 추가했습니다.🙋🏻 리뷰 시 주의 깊게 확인해야 하는 코드
📎 Issue 번호