Conversation
| if (chattingParticipantService.checkSubscription(chattingRoom, members)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
중복 구독 시 DuplicateSubscribeException 예외를 던지는 대신 return 하는 이유가 궁금합니다!
There was a problem hiding this comment.
private void checkDuplicateSubscription(ChattingParticipant chattingParticipant) {
if (chattingParticipant.getStatus() == Status.ACTIVE) {
throw new DuplicateSubscribeException();
}
}
중복 구독시 해당 메서드에서 예외를 날려주고 있습니다!
|
|
||
| public interface ChattingMessageRepository extends MongoRepository<ChattingMessage, String> { | ||
| Slice<ChattingMessage> findAllByRoomIdAndTimeStampAfterOrderByTimeStampDesc(Long roomId, LocalDateTime joinedAt, Pageable pageable); |
There was a problem hiding this comment.
roomId랑 timeStamp 필드를 기준으로 데이터를 검색하고 내림차순으로 정렬 할때 $gt로 greater than이랑 $lt로 less than를 표현해주는 방식은 처음 봤는데 많이 배웠습니다
| @@ -1,5 +1,5 @@ | |||
| package com.gachtaxi.domain.chat.entity.enums; | |||
|
|
|||
| public enum MessageType { | |||
| MESSAGE, ENTER | |||
| MESSAGE, ENTER, EXIT | |||
There was a problem hiding this comment.
MESSAGE만 존재하는 것이 아닌, MessageType 으로 ENTER와 EXIT를 지정해주신 이유는 채팅방 입장/퇴장 이벤트를 처리해주기 위한 설계로 이해했는데 맞을까요 ??
There was a problem hiding this comment.
넵 입장 메시지와 퇴장 메시지는 최초 입장, 채탕방 완전 퇴장 시에만 별도의 UI로 표시가 되니까 클라이언트 측에서 구분이 될 수 있도록 타입을 지정을 해뒀습니다
|
|
||
| // ChattingParticipant chattingParticipant = ChattingParticipant.of(chattingRoom, members); | ||
| // chattingParticipantRepository.save(chattingParticipant); | ||
| if (chattingParticipantService.checkSubscription(chattingRoom, members)) { |
There was a problem hiding this comment.
방 구독이랑 퇴장과 같은 메서드를 구분해서 구현해준 점은 좋지만, 해당 subscribeChatRoom 메서드에서 checkSubscription을 호출할 때, 메서드 내부적으로 findByChattingRoomAndMembers를 통해 구독 상태를 조회하는 과정에서 중복되는 쿼리 호출이 생겨서 성능에 지장이갈 수도 있다고 판단했습니다.
현재 API가 채팅과 같이 트래픽이 많은 API여서 최적화가 더 필요하다고도 생각되었습니다
Optional<ChattingParticipant> optionalParticipant = chattingParticipantRepository.findByChattingRoomAndMembers(chattingRoom, members);if문 밖에서 이렇게 미리 구독 상태를 조회한 뒤, 결과를 재사용하는 방식의 설계는 어떻게 생각하시는지 궁금합니다 !
There was a problem hiding this comment.
어떤 부분에서 중복 쿼리가 생기는지 다시 말씀해주실 수 있으실까요??
현재 구현된 상태에서는 멤버, 채팅방, 참여자 이렇게 3개의 select 쿼리가 날라갑니다!
말씀해 주신게 아래 코드라고 이해해서 쿼리를 비교해봤는데, 완전히 동일한 쿼리가 날라 갔습니다!
Optional<ChattingParticipant> optionalParticipant = chattingParticipantRepository.findByChattingRoomAndMembers(chattingRoom, members);
if (chattingParticipantService.checkSubscription(optionalParticipant)) {
return;
}
제가 이해한 부분이 다를 수 있으니 알려주시면 다시 확인해보겠습니다!
There was a problem hiding this comment.
아아 확인했습니다 ! subscribeChatRoom 메서드 내부에서 별도로 쿼리를 호출한 후 checkSubscription 내부에서도 동일한 쿼리를 호출할 가능성에 대해서 얘기했었던거였는데, 동일한 쿼리의 형식으로 작동한다면 문제가 없을거같습니다
| @@ -24,10 +28,42 @@ public class ChattingParticipant extends BaseEntity { | |||
| @JoinColumn(name = "members_id") | |||
| private Members members; | |||
|
|
|||
| @Builder.Default | |||
| @Enumerated(EnumType.STRING) | |||
| private Status status = Status.ACTIVE; | |||
There was a problem hiding this comment.
저희 서비스에 Status로 상태 관리를 하는 도메인이 많은데, 명확성을 위해 ChatStatus라는 이름으로 정의하면 어떨까요?
There was a problem hiding this comment.
해당 클래스가 채팅, 채팅방, 참여자 모두 사용 중이라 클래스 명만 ChatStatus로 수정하고, 내부 변수명은 status로 수정하는 건 어떠신가요??
# Conflicts: # src/main/java/com/gachtaxi/global/common/exception/handler/GlobalExceptionHandler.java
📌 관련 이슈
관련 이슈 번호 #28
Close #
🚀 작업 내용
📸 스크린샷
📢 리뷰 요구사항