Feat/#36 채팅 도메인 구현 #39

coke98 · 2024-02-02T09:01:18Z

🤨 Motivation

채팅 도메인 구현 #36

🔑 Key Changes

Stomp 설정
웹소켓 테스트 추가

🙏 To Reviewers

수현님 SuccessCode 커밋 체리픽했어요

…, 레포지토리, 카테고리 이넘 추가

mjj111

라이브 리뷰에서 작성된 사항들입니다!

mjj111 · 2024-02-07T04:45:57Z

src/main/java/com/aliens/backend/chat/controller/ChatBlockController.java

+    @PostMapping("/chat/block")
+    public ResponseEntity<Map<String, String>> blockPartner(@Login Long memberId, @RequestBody ChatBlockRequest chatBlockRequest) {
+        String result = chatBlockService.blockPartner(memberId, chatBlockRequest);
+        Map<String, String> response = Collections.singletonMap("response", result);


추후에 수현님께서 작성하신 SuccessResponse 사용해서 작성하면 좋을 것 같습니다!

ResponseEntity SuccessCode 반환, 어렵지 않으니, 제가 한 커밋 보시고 따라 하시면 됩니다 !

mjj111 · 2024-02-07T04:46:32Z

src/main/java/com/aliens/backend/chat/controller/ChatController.java

+
+    @GetMapping("/chat/room/{roomId}/messages")
+    public ResponseEntity<Map<String, List>> getMessages(@PathVariable("roomId") Long chatRoomId,
+                                                         @RequestParam(value = "lastMessageId", required = false) String lastMessageId) {


lastMessageId 는 필수가 아닌 이유가 있나요?!

이전 메시지들에 대한 정보를 가져오며 페이징(lastMessageId를 기준)을 사용할지,
가장 최근 100개를 가져올지
에 대해 2가지 옵션을 사용할 수 있도록 파라미터의 유무에 따라 나눌 수 있도록 구현했습니다

mjj111 · 2024-02-07T06:05:01Z

src/main/java/com/aliens/backend/chat/domain/ChatRoom.java

+    @Column
+    private Long id;
+    private Long roomId;
+    @Column(name = "member_id")


해당 Member에 대한 join이 필요해보입니다!

넵 join 적용해놓겠습니다!

mjj111 · 2024-02-07T06:06:14Z

src/main/java/com/aliens/backend/chat/domain/Message.java

+
+@Document(collection = "message")
+public class Message {
+    @Id


해당 아이디는 시퀀서를 통해 넣어지나요?

시퀀서를 사용하여 아이디를 생성하면, 데이터베이스 접근이 불가피하게 늘어나게 됩니다. 추가적인 구현없이 몽고 DB의 objectId를 사용하고 있어요
ObjectId에는 생성된 시각이 포함되기에 sort를 통해 만들어진 시간순으로 정렬이 가능합니다

mjj111 · 2024-02-07T06:08:51Z

src/main/java/com/aliens/backend/chat/domain/repository/CustomMessageRepositoryImpl.java

+
+@Repository
+public class CustomMessageRepositoryImpl implements CustomMessageRepository {
+    private final MongoTemplate mongoTemplate;


MongoTemplate과 MongoRepository를 같이 사용하고 계신데
MongoTemplate를 사용하신 이유가 있으신가요?

MongoRepository의 기본적인 CRUD(선언적인 접근 방식) 및 기능들을 사용하되,
다중 업데이트와 같이 쿼리 최적화가 필요한 부분을 위해 CustomMessageRepositoryImp내에서 mongoTemplete을 활용하도록 구현하였습니다. MongoRepository는 단일로만 쿼리가 전송이 되기에 쿼리 최적화가 어렵더라구요

결과적으로 messageRepository는 mongoRepository와 CustomMessageRepository를 모두 상속받아 몽고 레포지토리, 몽고 템플릿 기능 모두 활용할 수 있도록 구성했습니다!

mjj111 · 2024-02-07T06:14:38Z

src/main/java/com/aliens/backend/chat/controller/ChatController.java

+    }
+
+    @GetMapping("chat/summaries")
+    public ResponseEntity<Map<String, ChatSummaryResponse>> getChatSummaries(@Login Long memberId) {


@Login은 Token으로부터 Long 값인 MemberId만을 갖는게 아니라 LoginMember 객체를 가져옵니다!
수정이 필요해보입니다!

mjj111 · 2024-02-07T06:17:56Z

src/main/java/com/aliens/backend/global/interceptor/ChatChannelInterceptor.java

+@Component
+public class ChatChannelInterceptor implements ChannelInterceptor {
+
+    ChatAuthValidator chatAuthValidator;


private final 이 필요하지 않을까요??

"+ private final"
i know right

mjj111 · 2024-02-07T06:19:32Z

src/main/java/com/aliens/backend/global/interceptor/ChatHandshakeInterceptor.java

+    private final TokenProvider tokenProvider;
+    private final ChatService chatService;
+
+    public ChatHandshakeInterceptor(TokenProvider tokenProvider, @Lazy ChatService chatService) {


@lazy를 건 이유가 있을까요??

인터셉터단에서 서비스 빈 주입을 받는 순간 순환 참조가 일어나 컨텍스트 빌드가 실패하게됩니다. 실제 사용시에 의존성 주입을 하게 늦춰서 컨텍스트가 정상적으로 빌드 될 수 있도록 하고 있어용

coke98 · 2024-02-07T10:05:34Z

src/test/java/com/aliens/backend/chatting/WebSocketTest.java

+        //when
+        session.send(properties.getAppDestinationPrefix()+"/send", jsonMessage.getBytes());
+        //then
+        verify(chatController, timeout(100).times(1)).sendMessage(messageSendRequest);


해당 부분은 비동기 처리가 되다보니, 타임아웃을 설정하지 않으면 환경에따라 메서드 호출보다 검증이 더 빨라서 실패하더라구요. 테스트가 일관적이지 않을 수 있다는 단점이 있을 것 같다만 해당 방법도 괜찮을까요?

타임아웃이 일반적인 방법이라고 하는것 같은데, 다른 방법이라면...
CountDownLatch << 라는 녀석이 있다고 합니다.
이게 특정 조건이 충족될 때까지 테스트를 대기시키는 방법이라네요.
(비동기 작업이 완료될 때까지 테스트를 대기시키는 역할을 한다고 합니다. 근데 여기 적용할 수 있는진 모르겠네요..)

충분한 타임아웃이라면 저는 타임아웃도 좋은 방법이라고 생각합니다.

suhyun0918

현석님 코드 잘 봤습니다 !
여러 고민의 흔적이 많이 보였습니다 정말 고생많으셨습니다.

메서드 분리, 깔끔한 접근 제어자, 네이밍 등등 저도 영감을 얻어가는 부분들이 많았습니다 ㅎㅎ

제가 채팅 분야는 지식이 짧아 많은 도움 드리지 못했던 것 같아 아쉬웠습니다..

테스트 타임아웃 저도 의견아닌 의견(?) 드렸어요 ㅎㅎ

고생많으셨어요 ! 리뷰 확인해주세요~

suhyun0918 · 2024-02-07T09:59:21Z

src/main/java/com/aliens/backend/chat/domain/ChatBlock.java

+    public static ChatBlock of(Long blockingMemberId, ChatBlockRequest chatBlockRequest) {
+        ChatBlock chatBlock = new ChatBlock();
+        chatBlock.blockedMemberId = chatBlockRequest.partnerId();
+        chatBlock.blockingMemberId = blockingMemberId;
+        return chatBlock;
+    }


setter를 사용해서 객체를 완성하기 보단, 한 코드라인으로 할 수 있도록 return new 를 사용해서 객체를 반환하도록 하는건 어떨까요? 😆

good 이부분 왜 이렇게 했을까요 ㅎㅎ...

suhyun0918 · 2024-02-08T02:56:56Z

src/main/java/com/aliens/backend/chat/controller/ChatBlockController.java

+    @PostMapping("/chat/block")
+    public ResponseEntity<Map<String, String>> blockPartner(@Login Long memberId, @RequestBody ChatBlockRequest chatBlockRequest) {
+        String result = chatBlockService.blockPartner(memberId, chatBlockRequest);
+        Map<String, String> response = Collections.singletonMap("response", result);


ResponseEntity SuccessCode 반환, 어렵지 않으니, 제가 한 커밋 보시고 따라 하시면 됩니다 !

suhyun0918 · 2024-02-08T02:57:40Z

src/main/java/com/aliens/backend/chat/controller/ChatController.java

+import java.util.Map;
+
+
+@Controller


혹시, RestController가 아닌, Controller를 쓰신 이유가 있으실까요!

https://dncjf64.tistory.com/288

여기 두 어노에티션 간의 차이점이 잘 나와있어요 😊

아하! 사실 처음엔 MessageMapping이 Http요청이 아니다 보니 RestController를 사용하지 않았는데요..!
알아보니 RestController를 사용해도 웹소켓 쪽 MessageMapping에는 독립적이라 영향이 없더라구요.

추가적으로 다른 컨트롤러도 Controller를 덜컥.. 사용하고 있는데 테스트도 잘 통과한다고 놔뒀던 게, 둘간의 차이를 잘 이해하지 못하고 작성한 부분이었네요

RestController와 Controller의 정확한 차이는 덕분에 잘 알았습니다!
RestController로 변경하거나, responseBody를 사용하는 메서드에만 따로 붙이는 방법 고려해볼게요
꼼꼼히 봐주셨군요~ 감사힙니다

suhyun0918 · 2024-02-08T03:00:47Z

src/main/java/com/aliens/backend/chat/controller/ChatController.java

+    @MessageMapping("/send")
+    public void sendMessage(@Payload MessageSendRequest messageSendRequest) {
+        chatService.sendMessage(messageSendRequest);
+    }
+
+    @MessageMapping("/read")
+    public void readMessage(@Payload ReadRequest readRequest) {
+        chatService.readMessages(readRequest);
+    }


Chat의 Controller인데, url 매핑이 어느건 /chat 으로 시작하고, 어느건 그렇지 않은 것 같아요.

https://www.notion.so/RE-API-1d654705d6ab416592cef7eee1cb0c1c?pvs=4
저희 API 문서와 조금 다른 것 같은데, 의도가 있으신 걸까요 ?
😇

MessageMapping으로 들어오는 웹소켓 요청의 경우는 WebsocketConfig 쪽에서 요청을 분리하기위해 url에대한 prefix를 지정하는데요. prefix를 설정해준다면 /{prefix}/send로 요청을 보내야합니다.

그래서 prefix를 chat으로 지정해두었고
현재코드를 기준으로 결과적으로는 클라이언트에서는 다른 api요청과 같이 /chat/send url 요청을 하게됩니다.😃
수현님이 느끼셨던 것 처럼 조금 더 일관성 있게 작성하는게 더 좋아보이긴해요.
해당 부분은 url은 컨트롤러에서 모두 할당하고, prefix 없이 사용해도 문제가 없는지 확인해볼게요!

그리고 단일 메시지 읽음 -> 채팅방 전체 메시지 읽음으로 통합되면서 문서 업데이트가 안되었네요 해당 부분도 업데이트 하겠습니다!

suhyun0918 · 2024-02-08T03:01:51Z

src/main/java/com/aliens/backend/chat/controller/ChatController.java

+        chatService.readMessages(readRequest);
+    }
+
+    @GetMapping("chat/summaries")


@GetMapping("/chat/summaries") 로, chat 앞에 '/'가 빠져있네요 !
수정하는게 좋아보여요 😁

oh.. 바로 수정할게요
getMapping이라는 분은 서울에 사시는군요😃

suhyun0918 · 2024-02-08T03:36:00Z

src/main/java/com/aliens/backend/chat/service/ChatService.java

+    public List<Message> getMessages(Long chatRoomId, String lastMessageId) {
+        List<Message> messages = findMessages(chatRoomId, lastMessageId);
+        return messages;
+    }
+
+    public List<ChatRoom> getChatRooms(Long memberId) {
+        return chatRoomRepository.findByMemberId(memberId);
+    }
+
+    private void saveMessage(Message message) {
+        messageRepository.save(message);
+    }
+
+    private void publishMessage(Message message, Long ChatRoomId) {
+        messagingTemplate.convertAndSend("/room/"+ ChatRoomId, message);
+    }
+
+    private void sendNotification(Message message) {
+    }
+
+    private void updateReadState(Long chatRoomId, Long readBy) {
+        messageRepository.markMessagesAsRead(chatRoomId, readBy);
+    }
+
+    private void publishReadState(Long chatRoomId, Long memberId) {
+        ReadResponse readResponse = new ReadResponse(memberId);
+        messagingTemplate.convertAndSend("/room/" + chatRoomId, readResponse);
+    }
+
+    private List<Message> findMessages(Long chatRoomId, String lastMessageId) {
+        return messageRepository.findMessages(chatRoomId,lastMessageId);
+    }


저라면 코드라인 한줄짜리는 메서드화 하지 않았을 것 같은데,
이렇게 나뉜걸 보니 기능 별로 확실하게 알 수 있어서 더욱 보기 좋네요 ! 저도 본 받도록 해야겠습니다.
😍

함수를 나누는 기준은 다를 수 있지만, 저는 추상화 수준에 따라 나누었어요.😃

메서드: 채팅 메시지를 보낸다.(제일 높은 추상화 수준 so, Public){

저장한다(메서드)

발행한다(메서드)

알림을 보낸다(메서드)
}

이렇게 하면 채팅을 보내는 메서드가 어떤 행위를 할것인지에 대해 파악이 쉽기도 하고.
구현하는 단계에서도 문서 작성처럼 퍼블릭 메서드만 우선 작성해놓구, 한단계씩 private를 구현해가며 개발이 가능한데 해야할일이 명확해져서 좋더라구요.

suhyun0918 · 2024-02-08T03:39:07Z

src/main/java/com/aliens/backend/global/config/WebSocketConfig.java

+@EnableWebSocketMessageBroker
+public class WebSocketConfig implements WebSocketMessageBrokerConfigurer {
+
+    private final WebSocketProperties properties;


properties 나눈거 좋아요 ㅎㅎ 🤗

suhyun0918 · 2024-02-08T03:42:33Z

src/main/java/com/aliens/backend/global/interceptor/ChatChannelInterceptor.java

+@Component
+public class ChatChannelInterceptor implements ChannelInterceptor {
+
+    ChatAuthValidator chatAuthValidator;


"+ private final"
i know right

suhyun0918 · 2024-02-08T03:54:23Z

src/main/java/com/aliens/backend/chat/service/ChatAuthValidator.java

+public class ChatAuthValidator {
+    public void validateRoom(String topic, List<ChatRoom> validChatRooms) {
+        Long roomId = getRoomIdFromTopic(topic);
+        if(!isValidRoom(validChatRooms, roomId)) {
+            throw new RestApiException(ChatError.INVALID_ROOM_ACCESS);
+        }
+    }
+
+    private boolean isValidRoom (List<ChatRoom> validChatRooms, Long roomId) {
+        return validChatRooms.stream().anyMatch(chatRoom -> chatRoom.getRoomId().equals(roomId));
+    }
+
+    private long getRoomIdFromTopic(String topic) {
+        return Long.parseLong(topic.split("/")[1]);
+    }
+}


명준님이 제게 코드 리뷰 해주셨을때, Room에 대해서 검증을 하는 거라면 Validator 클래스 보다는
Room 엔티티에서 검증을 하는게 어떻냐고 말씀해주시더라구요. 각자 본인이 검증을 하도록 변경하는것도 고려해보시면 어떨까요?
🤗

음 이 부분도 고민해볼게요! validChatRooms를 일급 컬렉션으로 만들고 isvalid를 메서드로 두는 방법도 있을지두

suhyun0918 · 2024-02-08T04:23:38Z

src/test/java/com/aliens/backend/chatting/WebSocketTest.java

+        //when
+        session.send(properties.getAppDestinationPrefix()+"/send", jsonMessage.getBytes());
+        //then
+        verify(chatController, timeout(100).times(1)).sendMessage(messageSendRequest);


타임아웃이 일반적인 방법이라고 하는것 같은데, 다른 방법이라면...
CountDownLatch << 라는 녀석이 있다고 합니다.
이게 특정 조건이 충족될 때까지 테스트를 대기시키는 방법이라네요.
(비동기 작업이 완료될 때까지 테스트를 대기시키는 역할을 한다고 합니다. 근데 여기 적용할 수 있는진 모르겠네요..)

충분한 타임아웃이라면 저는 타임아웃도 좋은 방법이라고 생각합니다.

…l로 변경

…roller를 RestController로 변경

mjj111

확인했습니다~! 다음 이슈 때 변경 부탁 드리겠습니다!

coke98 added feat 기능 개발 이슈 test 테스트 관련 태그 labels Feb 2, 2024

coke98 requested review from mjj111 and suhyun0918 February 2, 2024 09:01

coke98 self-assigned this Feb 2, 2024

coke98 changed the base branch from main to develop February 2, 2024 09:01

coke98 changed the title ~~#36 chatting~~ Feat/#36 채팅 도메인 구현 Feb 2, 2024

coke98 added the chore 빌드 관련 태그 label Feb 2, 2024

suhyun0918 and others added 14 commits February 3, 2024 22:37

feat(SuccessCode) : 성공 코드, 메시지를 담을 수 있도록 커스텀 ResponseEntity 정의

61ffab3

chore(build.gradle): 웹소켓 의존성 추가

efc2ea9

feat(WebSocketProperties): 웹소켓 Properties 추가

532ada4

feat(ChatClient): STOMP 테스트를 위한 채팅 클라이언트 추가

4d9fd70

test(WebSocketTest): 웹소켓 테스트 추가

0b84acd

feat(WebSocketConfig): 웹소켓 설정 클래스 추가

f90f128

test(WebSocketTest): 메시지 전송, 읽음 처리, 채팅 요약정보 요청 케이스 추가

a405a6d

feat(ChatController): 테스트를 위한 컨트롤러, 서비스 클래스 추가

04749c1

chore(build.gradle): mongo db를 위한 의존성 추가

5d5f100

feat(Message,MessageRepository): 메시지 Document와 레포지토리 추가

8d31222

feat(ChatSummary): 채팅 요약 정보 DTO 추가

ee38920

test(ChatServiceTest):채팅 서비스 테스트 추가

e679c0d

feat(ChatSuccessCode): 채팅 성공 코드 추가

4d97f4d

Merge remote-tracking branch 'origin/develop' into #36_chatting_merge

4cf83df

coke98 force-pushed the #36_chatting branch from efe3794 to 4cf83df Compare February 4, 2024 05:44

coke98 added 7 commits February 6, 2024 01:01

feat(MessageSendRequest): 메시지 전송 요청 레코드 추가

5d9962e

refactor(ChatSummary): 패키지 이동

7909c3d

test(ChatServiceTest,WebsocketTest): 메시지 전송 테스트 코드 Payload 타입 수정

298941b

feat(Message): 빌더 삭제, 팩토리 메서드 추가

d5cc10d

feat(MessageType): 메시지 타입 Enum fromString메서드 추가

8ddeb64

feat(ChatError): 채팅 에러 코드 추가

4a8bc7d

feat(ChatController): 메시지 전송 Payload 수정

a308c20

coke98 added 19 commits February 7, 2024 05:05

feat(ChatService): 채팅방 목록 조회 메서드 추가

64d35fa

feat(ChatChannelInterceptor): 채팅방 구독시 인가 검증 구현

ed11b91

feat(ChatAuthValidator): 채팅 권한 검증 클래스 추가

3640499

feat(ChatError): 채팅방 접근 권한 없음 에러 코드 추가

6bd0da8

refactor(repository): 패키지, 테이블명 오타 수정

232143a

test(ChatDocTest): 채팅 신고 테스트 코드 추가

81c8c41

feat(ChatReportController): 채팅 신고 컨트롤러 추가

ae98257

feat(ChatReportService): 채팅 신고 서비스 추가

d667243

feat(ChatReportRequest): 채팅 신고 요청 레코드 추가

b9371bc

feat(ChatReport, ChatReportCategory, ChatReportRepository): 채팅 신고 엔티티…

13909b9

…, 레포지토리, 카테고리 이넘 추가

feat(ChatError): INVALID_REPORT_CATEGORY 에러코드 추가

8b9a467

feat(ChatDocTest): 채팅 차단 테스트 코드 추가

b9c04fc

feat(ChatBlockController, ChatBlockRequest): 채팅 차단 컨트롤러 추가, 차단 요청 레코드 추가

ecb842a

feat(ChatBlockService): 채팅 차단 서비스 추가

1897b21

feat(ChatBlock, ChatBlockRepository): 채팅 차단 엔티티, 레포지토리 추가

2290eb3

feat(ChatRoom): 채팅방을 차단하는 block 메소드 추가

2f51ba5

feat(ChatError, ChatSuccessCode): 에러 및 성공 코드 추가

9853e39

feat(ChatRoom): 채팅방 식별을 위해 id 대신 roomId를 사용하도록 변경

818505d

feat(ChatBlockServiceTest, ChatReportServiceTest): 채팅 신고, 차단 서비스 테스트 추가

2e5d563

mjj111 requested changes Feb 7, 2024

View reviewed changes

coke98 commented Feb 7, 2024

View reviewed changes

suhyun0918 approved these changes Feb 8, 2024

View reviewed changes

coke98 added 4 commits February 8, 2024 17:25

chore(application.yml): 웹소켓 설정 추가

23ca031

feat(ChatChannelInterceptor): ChatAuthValidator의 접근 제한자를 private fina…

4769400

…l로 변경

feat(ChatBlockController, ChatController, ChatReportController): Cont…

8825189

…roller를 RestController로 변경

feat(ChatController): 경로 수정

208aa59

mjj111 approved these changes Feb 10, 2024

View reviewed changes

Merge branch 'develop' into #36_chatting

73bdbc2

mjj111 merged commit 462b515 into develop Feb 10, 2024
1 check failed

coke98 mentioned this pull request Feb 15, 2024

Feat/#36 채팅 도메인 리팩토링 #63

Merged

		import java.util.Map;


		@Controller

Feat/#36 채팅 도메인 구현 #39

Feat/#36 채팅 도메인 구현 #39

Conversation

coke98 commented Feb 2, 2024

🤨 Motivation

🔑 Key Changes

🙏 To Reviewers

mjj111 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suhyun0918 Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coke98 Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coke98 Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suhyun0918 Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suhyun0918 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suhyun0918 Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coke98 Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coke98 Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coke98 Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suhyun0918 Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjj111 left a comment

Choose a reason for hiding this comment

suhyun0918 Feb 8, 2024 •

edited

Loading

coke98 Feb 8, 2024 •

edited

Loading

coke98 Feb 8, 2024 •

edited

Loading

suhyun0918 Feb 8, 2024 •

edited

Loading

suhyun0918 Feb 8, 2024 •

edited

Loading

coke98 Feb 8, 2024 •

edited

Loading

coke98 Feb 8, 2024 •

edited

Loading

coke98 Feb 8, 2024 •

edited

Loading

suhyun0918 Feb 8, 2024 •

edited

Loading