[COZY-665] feat : 채팅 기능 구현 #374
Conversation
momuzzi
requested review from
eple0329,
genius00hwan and
suuu0719
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/cozymate/cozymate_server/domain/notificationlog/enums/NotificationType.java (1)
302-314: NotificationLogCreateDTO에 chatRoomId 필드 및 getter 추가 후 ARRIVE_CHAT 로직 수정
- src/main/java/com/cozymate/cozymate_server/domain/notificationlog/dto/NotificationLogCreateDTO.java:
private Long chatRoomId추가, 생성자·@builder·@Getter에 반영- src/main/java/com/cozymate/cozymate_server/domain/notificationlog/enums/NotificationType.java (ARRIVE_CHAT):
generateNotificationLog에서createDTO.getChatRoomId()사용하도록 변경
🧹 Nitpick comments (23)
src/main/java/com/cozymate/cozymate_server/domain/chat/dto/redis/ChatPubDTO.java (1)
13-14: createdAt 시간 차이에 대한 문서화 개선Line 13의 주석에서 "DB에 저장될 createdAt와 약간의 오차 있음"이라고 명시되어 있습니다. 이 시간 차이가:
- Redis Pub 시점과 MongoDB 저장 시점의 차이인지
- 비즈니스 로직상 허용 가능한 수준인지
- 클라이언트에 영향을 미치는지
명확히 문서화하는 것이 좋습니다. 또한 Line 14의 "pub sequence는 0 고정" 주석도 이유를 추가하면 유지보수에 도움이 됩니다.
src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoomMember.java (1)
28-32: 외래 키 제약 조건 추가를 고려하세요.
chatRoom과member필드에@JoinColumn을 추가하여 외래 키 이름과 제약 조건을 명시하는 것이 좋습니다. 특히nullable = false를 설정하여 데이터 무결성을 보장할 수 있습니다.예시:
@ManyToOne(fetch = FetchType.LAZY) +@JoinColumn(name = "chat_room_id", nullable = false) private ChatRoom chatRoom; @ManyToOne(fetch = FetchType.LAZY) +@JoinColumn(name = "member_id", nullable = false) private Member member;src/main/java/com/cozymate/cozymate_server/global/redispubsub/event/StompSubEvent.java (1)
3-5: 필드 검증 추가를 고려하세요.
chatRoomId가 null이거나 유효하지 않은 값일 경우를 방지하기 위해@NotNull또는 생성자에서 검증을 추가하는 것을 권장합니다. 이벤트 처리 중 NPE를 방지할 수 있습니다.예시:
import jakarta.validation.constraints.NotNull; public record StompSubEvent( @NotNull Long chatRoomId ) { }src/main/java/com/cozymate/cozymate_server/domain/member/converter/MemberConverter.java (1)
96-108: 하드코딩 제거 및 도메인 의미 확인 필요
- "(알수없음)" 문자열, persona=1은 매직값입니다. 의미를 명확히 하기 위해 상수로 추출해 주세요.
- persona=1이 “탈퇴/미상”을 의미하는 공식 값인지 확인이 필요합니다. 아니라면 캐시에서 null 허용 또는 전용 sentinel을 고려해 주세요.
- 입력값(nickname/persona)에 대한 유효성(예: 1~16 범위) 보강 또는 상위 레이어에서 보장해 주세요.
적용 예(해당 메서드 내부 치환):
- .nickname("(알수없음)") - .persona(1) + .nickname(UNKNOWN_NICKNAME) + .persona(UNKNOWN_PERSONA)클래스 상단에 상수 추가(예시):
private static final String UNKNOWN_NICKNAME = "(알수없음)"; private static final int UNKNOWN_PERSONA = 1; // 도메인 의미 확인 필요src/main/java/com/cozymate/cozymate_server/global/redispubsub/config/RedisPubSubConfig.java (2)
18-21: MessageListenerAdapter 기본 메서드 지정 확인Spring Data Redis 기본값은 "handleMessage"입니다. RedisPubSubListener가 onMessage(Message, byte[])만 구현했다면 호출 불일치로 런타임 문제 가능성이 있습니다. 명시 지정 또는 어댑터 미사용(직접 MessageListener 등록) 중 하나를 권장합니다.
- return new MessageListenerAdapter(redisPubSubListener); + return new MessageListenerAdapter(redisPubSubListener, "onMessage");
24-28: 컨테이너에 리스너/토픽 등록 경로 확인 및 에러 핸들러 권장현재 컨테이너는 커넥션만 설정합니다. 런타임에 RedisSubscriberManager 등에서 addMessageListener(adapter, topic)을 호출하는지 확인 부탁드립니다. 또한, 리스너 예외가 삼켜지지 않도록 ErrorHandler 설정을 권장합니다.
예시:
container.setErrorHandler(t -> /* 로깅/모니터링 */ );src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (1)
10-23: 조회 성능을 위한 복합 인덱스 제안(chatRoomId, createdAt, sequence)findChatsInRange 패턴에 맞춰 복합 인덱스를 추가하면 스캔을 줄일 수 있습니다.
+import org.springframework.data.mongodb.core.index.CompoundIndex; +import org.springframework.data.mongodb.core.index.CompoundIndexes; @@ -@Document("chat") +@CompoundIndexes({ + @CompoundIndex(name = "chat_room_time_seq", + def = "{'chatRoomId': 1, 'createdAt': 1, 'sequence': 1}") +}) +@Document("chat") public class Chat {src/main/java/com/cozymate/cozymate_server/domain/chat/repository/ChatRepositoryService.java (2)
16-18: 저장 결과 반환 고려식별자/생성시간 등 후속 처리에 유용하므로 저장 결과를 반환하는 편이 일반적입니다.
- public void saveChat(Chat chat) { - chatRepository.save(chat); - } + public Chat saveChat(Chat chat) { + return chatRepository.save(chat); + }
20-24: PageRequest 대신 Pageable 수용으로 API 유연성 향상서비스 계층에서 PageRequest에 고정될 필요가 없습니다. Pageable을 받으면 호출자 선택지를 넓힐 수 있습니다.
- public List<Chat> getChatListByRange(Long chatRoomId, LocalDateTime enterTime, - LocalDateTime lastChatTime, Long sequence, PageRequest pageRequest) { + public List<Chat> getChatListByRange(Long chatRoomId, LocalDateTime enterTime, + LocalDateTime lastChatTime, Long sequence, Pageable pageable) { - return chatRepository.findChatsInRange(chatRoomId, enterTime, lastChatTime, sequence, - pageRequest); + return chatRepository.findChatsInRange(chatRoomId, enterTime, lastChatTime, sequence, + pageable); }src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (1)
391-401: 배치 구간 계산 단순화(가독성)end 계산을 Math.min으로 단순화하면 가독성이 좋아집니다.
- for (int start = 0; start < fcmSqsMessageList.size(); start += BATCH_SIZE) { - int end = start + BATCH_SIZE; - - if (end > fcmSqsMessageList.size()) { - end = fcmSqsMessageList.size(); - } - - batchList.add(fcmSqsMessageList.subList(start, end)); - } + for (int start = 0; start < fcmSqsMessageList.size(); start += BATCH_SIZE) { + int end = Math.min(start + BATCH_SIZE, fcmSqsMessageList.size()); + batchList.add(fcmSqsMessageList.subList(start, end)); + }src/main/java/com/cozymate/cozymate_server/domain/chat/repository/ChatRepository.java (1)
12-26: 조회 정렬 보장 및 색인 권고
- 현재 쿼리는 정렬이 명시되지 않아 페이지네이션 시 순서가 비결정적일 수 있습니다(특히 동일 createdAt에서 sequence로 2차 정렬 필요).
- 정렬을 쿼리에 명시하고, 복합 인덱스를 추가하는 것을 권장합니다.
- @Query("{"+ + @Query(value = "{"+ "'chatRoomId': ?0," + "$or: [" + "{ 'createdAt': { $gt: ?1, $lt: ?2 } }," + "{ 'createdAt': ?2, 'sequence': { $lt: ?3 } }" + "]" + - "}") + "}", + sort = "{ 'createdAt': 1, 'sequence': 1 }") List<Chat> findChatsInRange(운영 조언:
- Mongo에
{ chatRoomId: 1, createdAt: 1, sequence: 1 }복합 인덱스를 추가해 주세요. 대화방별 범위 조회/정렬에 필수입니다.src/main/java/com/cozymate/cozymate_server/domain/chat/controller/ChatController.java (2)
35-40: Swagger 문서용 더미 엔드포인트의 목적을 명확히 하세요.이 엔드포인트가 Swagger 문서 생성을 위한 더미 엔드포인트임을 코드에 명시하는 것이 좋습니다. 실제 WebSocket을 통해 메시지를 보내야 함을 개발자가 명확히 인지할 수 있도록 주석이나 설명을 보강하세요.
+ // Note: 실제 채팅 전송은 WebSocket STOMP를 통해 /pub/chats로 전송해야 합니다. + // 이 엔드포인트는 Swagger 문서 생성을 위한 참고용입니다. @Operation(summary = "[베로] 채팅 전송 (docs)", description = "WebSocket STOMP로 채팅 전송") @GetMapping("/pub/chats")
42-51: 페이지네이션 파라미터 검증을 고려하세요.
lastChatTime과sequence가 필수 파라미터이지만, null이거나 잘못된 값(예: 음수 sequence)에 대한 검증이 없습니다. 서비스 레이어에서 검증하더라도 컨트롤러 레벨에서 기본적인 검증을 추가하는 것을 권장합니다.@GetMapping("/chats/chatrooms/{chatRoomId}") public ResponseEntity<ApiResponse<ChatListResponseDTO>> getChatList( @AuthenticationPrincipal MemberDetails memberDetails, @PathVariable Long chatRoomId, - @RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) LocalDateTime lastChatTime, - @RequestParam Long sequence) { + @RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) @NotNull LocalDateTime lastChatTime, + @RequestParam @NotNull @Min(0) Long sequence) {src/main/java/com/cozymate/cozymate_server/domain/chatroom/controller/ChatRoomController.java (1)
39-48: 채팅방 입장 엔드포인트의 HTTP 메서드를 재고하세요.채팅방 입장이 멤버십을 생성하거나 상태를 변경하는 경우 POST가 적절하지만, 단순히 최근 채팅을 조회하는 것이라면 GET이 더 의미적으로 맞을 수 있습니다.
enterChatRoom의 부수 효과(멤버 추가, 참가자 수 증가 등)를 확인하세요.부수 효과가 없다면 GET으로 변경을 고려하세요:
- @PostMapping("/{chatRoomId}") + @GetMapping("/{chatRoomId}/enter") public ResponseEntity<ApiResponse<ChatListResponseDTO>> enterChatRoom(src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketHandshakeInterceptor.java (1)
39-45: JWT 검증 예외 처리를 구체화하세요.Line 41에서 모든 예외를 포괄적으로 처리하고 있습니다. JWT 검증 실패의 구체적인 원인(만료, 서명 오류 등)을 구분하여 로깅하면 디버깅에 도움이 됩니다.
try { jwtUtil.validateToken(jwt); - } catch (Exception e) { - log.info("WebSocketHandShake 전 jwt 인증 실패"); + } catch (io.jsonwebtoken.ExpiredJwtException e) { + log.info("WebSocketHandShake 전 jwt 만료: {}", e.getMessage()); + response.setStatusCode(HttpStatus.UNAUTHORIZED); + return false; + } catch (io.jsonwebtoken.JwtException e) { + log.info("WebSocketHandShake 전 jwt 검증 실패: {}", e.getMessage()); + response.setStatusCode(HttpStatus.UNAUTHORIZED); + return false; + } catch (Exception e) { + log.error("WebSocketHandShake 전 예상치 못한 오류", e); response.setStatusCode(HttpStatus.UNAUTHORIZED); return false; }src/main/java/com/cozymate/cozymate_server/domain/chat/converter/ChatConverter.java (1)
34-34: LocalDateTime.now() 사용이 테스트 가능성을 저해합니다.변환기에서
LocalDateTime.now()를 직접 호출하면 단위 테스트 시 시간 제어가 어렵습니다. 시간을 파라미터로 받거나, Clock을 주입하는 것을 권장합니다.- public static ChatPubDTO toChatPubDTO(CreateChatRequestDTO createChatRequestDTO, - MemberCachingDTO memberCachingDTO) { + public static ChatPubDTO toChatPubDTO(CreateChatRequestDTO createChatRequestDTO, + MemberCachingDTO memberCachingDTO, LocalDateTime createdAt) { return ChatPubDTO.builder() .chatRoomId(createChatRequestDTO.chatRoomId()) .persona(memberCachingDTO.persona()) .memberId(createChatRequestDTO.memberId()) .nickname(memberCachingDTO.nickname()) .content(createChatRequestDTO.content()) - .createdAt(LocalDateTime.now()) + .createdAt(createdAt) .sequence(0l) .build(); }src/main/java/com/cozymate/cozymate_server/domain/sqs/service/SQSMessageCreator.java (1)
245-246: 메서드 시그니처 변경이 불필요하게 줄바꿈되었습니다.Line 245-246에서 메서드 파라미터가 변경되면서 불필요하게 줄바꿈된 것으로 보입니다. 일관성을 위해 다른 메서드와 동일한 스타일을 유지하세요.
private SQSMessageResult getMessageResultWithMessageRoomId(List<Fcm> fcmList, String content, - NotificationType notificationType, NotificationLog notificationLog, - MessageRoom messageRoom) { + NotificationType notificationType, NotificationLog notificationLog, MessageRoom messageRoom) {src/main/java/com/cozymate/cozymate_server/domain/chatroom/repository/ChatRoomRepositoryService.java (2)
11-15: 트랜잭션 경계 추가 제안 (읽기/쓰기 분리).Repository 래핑 서비스이므로 기본 readOnly 트랜잭션을 클래스에 부여하고, 쓰기 메서드에서만 readOnly 해제하는 편이 안전합니다.
다음 변경을 권장합니다:
import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Component; +import org.springframework.transaction.annotation.Transactional; @Component @RequiredArgsConstructor +@Transactional(readOnly = true) public class ChatRoomRepositoryService {
40-42: 쓰기 메서드에 @transactional 적용 및 반환형 개선 제안.
- save/delete 계열은 명시적으로 트랜잭션을 시작하는 것이 안전합니다.
- save 메서드는 저장된 엔티티를 반환하면 호출자가 id/상태를 활용하기 좋습니다.
적용 예:
- public void saveChatRoomMember(ChatRoomMember chatRoomMember) { - chatRoomMemberRepository.save(chatRoomMember); - } + @Transactional + public ChatRoomMember saveChatRoomMember(ChatRoomMember chatRoomMember) { + return chatRoomMemberRepository.save(chatRoomMember); + } - public void deleteAllChatRoomMemberByMemberId(Long memberId) { - chatRoomMemberRepository.deleteAllByMemberId(memberId); - } + @Transactional + public void deleteAllChatRoomMemberByMemberId(Long memberId) { + chatRoomMemberRepository.deleteAllByMemberId(memberId); + }Also applies to: 48-50
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamConsumer.java (2)
31-33: 동시성 안전한 Map 사용 권장.여러 스레드(스케줄러/초기화)에서 접근 가능성이 있으므로 ConcurrentHashMap으로 교체하세요.
- private final Map<Long, Subscription> dbSubscriptions = new HashMap<>(); + private final Map<Long, Subscription> dbSubscriptions = new java.util.concurrent.ConcurrentHashMap<>();
75-79: 중복 구독 방지 가드 추가.이미 구독 중인 chatRoomId에 대해 중복 등록을 피하세요.
- // 3. 서버 메모리 내에서 Subscription 관리 - dbSubscriptions.put(chatRoomId, dbSub); + // 3. 서버 메모리 내에서 Subscription 관리 (중복 구독 방지) + if (dbSubscriptions.putIfAbsent(chatRoomId, dbSub) != null) { + log.debug("ChatRoom {} 이미 구독 중입니다. 중복 구독을 건너뜁니다.", chatRoomId); + dbSub.cancel(); + return; + }src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java (1)
202-220: Map 조회 중복 최소화(가독성/할당 감소).동일 키에 대해 두 번 getOrDefault 호출하면서 기본 DTO를 매번 생성합니다. 한 번만 조회하고 로컬 변수로 재사용하세요.
- List<ChatResponseDTO> chatResponseDTOList = chatList.stream() - .map(chat -> ChatConverter.toChatResponseDTO(chat, - cachingMemberMap.getOrDefault(chat.getMemberId(), - MemberConverter.toWithdrawMemberCachingDTO()).nickname(), - cachingMemberMap.getOrDefault(chat.getMemberId(), - MemberConverter.toWithdrawMemberCachingDTO()).persona())) - .toList(); + final var defaultMember = MemberConverter.toWithdrawMemberCachingDTO(); + List<ChatResponseDTO> chatResponseDTOList = chatList.stream() + .map(chat -> { + MemberCachingDTO cached = cachingMemberMap.getOrDefault(chat.getMemberId(), defaultMember); + return ChatConverter.toChatResponseDTO(chat, cached.nickname(), cached.persona()); + }) + .toList();src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java (1)
243-254: Listener 컨테이너 스레드/옵션 하드코딩 — 프로퍼티화 권장.pollTimeout, batchSize, executor 스레드 수(현재 2)는 트래픽/방 수에 따라 튜닝 필요합니다. 설정값으로 외부화하거나 Executor 빈 주입을 권장합니다.
예시: @value로 thread 수/properties 주입 또는 TaskExecutor 빈을 주입받아 사용.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (58)
build.gradle(1 hunks)config(1 hunks)src/main/java/com/cozymate/cozymate_server/CozymateServerApplication.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/controller/ChatController.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/converter/ChatConverter.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/dto/redis/ChatPubDTO.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/dto/redis/ChatStreamDTO.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/dto/request/CreateChatRequestDTO.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/dto/response/ChatListResponseDTO.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/dto/response/ChatResponseDTO.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/repository/ChatRepository.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/repository/ChatRepositoryService.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamConsumer.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoom.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoomMember.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chatroom/controller/ChatRoomController.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chatroom/converter/ChatRoomConverter.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chatroom/dto/response/ChatRoomResponseDTO.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chatroom/repository/ChatRoomMemberRepository.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chatroom/repository/ChatRoomRepository.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chatroom/repository/ChatRoomRepositoryService.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chatroom/service/ChatRoomService.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/fcm/event/SentChatEvent.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/fcm/event/converter/EventConverter.java(2 hunks)src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java(6 hunks)src/main/java/com/cozymate/cozymate_server/domain/member/controller/MemberController.java(4 hunks)src/main/java/com/cozymate/cozymate_server/domain/member/converter/MemberConverter.java(2 hunks)src/main/java/com/cozymate/cozymate_server/domain/member/dto/MemberCachingDTO.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/member/repository/MemberRepositoryService.java(2 hunks)src/main/java/com/cozymate/cozymate_server/domain/member/service/MemberCacheService.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/member/service/MemberWithdrawService.java(3 hunks)src/main/java/com/cozymate/cozymate_server/domain/notificationlog/enums/NotificationType.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/sqs/dto/FcmSQSMessage.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/sqs/service/SQSMessageCreator.java(3 hunks)src/main/java/com/cozymate/cozymate_server/global/config/JpaConfig.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/config/MongoConfig.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/config/ObjectMapperConfig.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/config/SecurityConfig.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/config/SshRedisConfig.java(3 hunks)src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPubSubListener.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPublisher.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisSubscriberManager.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/redispubsub/config/RedisPubSubConfig.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/redispubsub/event/StompDisconnectEvent.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/redispubsub/event/StompSubEvent.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/response/code/status/ErrorStatus.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/response/exception/ExceptionAdvice.java(2 hunks)src/main/java/com/cozymate/cozymate_server/global/response/exception/WebSocketException.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/scheduler/RedisStreamScheduler.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/websocket/StompErrorHandler.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/websocket/StompInterceptor.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketEventListener.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketHandshakeInterceptor.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/websocket/config/WebSocketConfig.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/websocket/repository/WebSocketSessionRepository.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (28)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (3)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java (1)
Slf4j(38-221)src/main/java/com/cozymate/cozymate_server/domain/sqs/service/SQSMessageSender.java (1)
Slf4j(14-42)src/main/java/com/cozymate/cozymate_server/domain/sqs/service/SQSMessageCreator.java (1)
Component(18-306)
src/main/java/com/cozymate/cozymate_server/domain/chat/dto/response/ChatListResponseDTO.java (1)
src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (1)
Builder(10-23)
src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoomMember.java (2)
src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (1)
Builder(10-23)src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoom.java (1)
Getter(18-41)
src/main/java/com/cozymate/cozymate_server/global/websocket/StompErrorHandler.java (1)
src/main/java/com/cozymate/cozymate_server/global/websocket/config/WebSocketConfig.java (1)
Configuration(14-49)
src/main/java/com/cozymate/cozymate_server/domain/chat/dto/response/ChatResponseDTO.java (1)
src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (1)
Builder(10-23)
src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisSubscriberManager.java (2)
src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPubSubListener.java (1)
Component(14-34)src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPublisher.java (1)
Component(8-17)
src/main/java/com/cozymate/cozymate_server/domain/chatroom/service/ChatRoomService.java (2)
src/main/java/com/cozymate/cozymate_server/domain/chatroom/converter/ChatRoomConverter.java (1)
ChatRoomConverter(8-25)src/main/java/com/cozymate/cozymate_server/global/response/exception/ExceptionAdvice.java (1)
Slf4j(32-229)
src/main/java/com/cozymate/cozymate_server/domain/chatroom/controller/ChatRoomController.java (1)
src/main/java/com/cozymate/cozymate_server/domain/chat/controller/ChatController.java (1)
RestController(24-52)
src/main/java/com/cozymate/cozymate_server/domain/chatroom/repository/ChatRoomRepositoryService.java (2)
src/main/java/com/cozymate/cozymate_server/domain/chat/repository/ChatRepositoryService.java (1)
Component(10-25)src/main/java/com/cozymate/cozymate_server/domain/messageroom/repository/MessageRoomRepositoryService.java (1)
Component(15-46)
src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketEventListener.java (3)
src/main/java/com/cozymate/cozymate_server/global/websocket/StompInterceptor.java (1)
Slf4j(23-82)src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketHandshakeInterceptor.java (1)
Slf4j(21-63)src/main/java/com/cozymate/cozymate_server/global/websocket/repository/WebSocketSessionRepository.java (1)
Slf4j(12-112)
src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPubSubListener.java (1)
src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPublisher.java (1)
Component(8-17)
src/main/java/com/cozymate/cozymate_server/domain/notificationlog/enums/NotificationType.java (1)
src/main/java/com/cozymate/cozymate_server/domain/notificationlog/converter/NotificationLogConverter.java (1)
NotificationLogConverter(11-48)
src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (2)
src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoom.java (1)
Getter(18-41)src/main/java/com/cozymate/cozymate_server/domain/member/Member.java (1)
Getter(35-103)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/SentChatEvent.java (1)
src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (1)
Builder(10-23)
src/main/java/com/cozymate/cozymate_server/domain/chat/dto/redis/ChatStreamDTO.java (1)
src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (1)
Builder(10-23)
src/main/java/com/cozymate/cozymate_server/global/websocket/StompInterceptor.java (3)
src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketEventListener.java (1)
Slf4j(12-29)src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketHandshakeInterceptor.java (1)
Slf4j(21-63)src/main/java/com/cozymate/cozymate_server/global/websocket/repository/WebSocketSessionRepository.java (1)
Slf4j(12-112)
src/main/java/com/cozymate/cozymate_server/domain/chat/dto/redis/ChatPubDTO.java (1)
src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (1)
Builder(10-23)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java (3)
src/main/java/com/cozymate/cozymate_server/domain/chat/converter/ChatConverter.java (1)
ChatConverter(13-58)src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java (1)
Slf4j(38-221)src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamConsumer.java (1)
Slf4j(26-111)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamConsumer.java (3)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java (1)
Slf4j(38-221)src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java (1)
Slf4j(45-300)src/main/java/com/cozymate/cozymate_server/domain/chat/repository/ChatRepositoryService.java (1)
Component(10-25)
src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPublisher.java (1)
src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPubSubListener.java (1)
Component(14-34)
src/main/java/com/cozymate/cozymate_server/global/websocket/repository/WebSocketSessionRepository.java (3)
src/main/java/com/cozymate/cozymate_server/global/websocket/StompInterceptor.java (1)
Slf4j(23-82)src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketEventListener.java (1)
Slf4j(12-29)src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketHandshakeInterceptor.java (1)
Slf4j(21-63)
src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoom.java (3)
src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoomMember.java (1)
Getter(17-39)src/main/java/com/cozymate/cozymate_server/domain/university/University.java (1)
Getter(18-47)src/main/java/com/cozymate/cozymate_server/global/utils/BaseTimeEntity.java (1)
Getter(12-29)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java (6)
src/main/java/com/cozymate/cozymate_server/domain/chat/converter/ChatConverter.java (1)
ChatConverter(13-58)src/main/java/com/cozymate/cozymate_server/domain/fcm/event/converter/EventConverter.java (1)
EventConverter(18-98)src/main/java/com/cozymate/cozymate_server/domain/member/converter/MemberConverter.java (1)
MemberConverter(16-117)src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamConsumer.java (1)
Slf4j(26-111)src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java (1)
Slf4j(45-300)src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (1)
Slf4j(47-411)
src/main/java/com/cozymate/cozymate_server/global/websocket/config/WebSocketConfig.java (1)
src/main/java/com/cozymate/cozymate_server/global/websocket/StompErrorHandler.java (1)
Configuration(16-45)
src/main/java/com/cozymate/cozymate_server/domain/chat/controller/ChatController.java (1)
src/main/java/com/cozymate/cozymate_server/domain/chatroom/controller/ChatRoomController.java (1)
RestController(23-58)
src/main/java/com/cozymate/cozymate_server/domain/member/controller/MemberController.java (1)
src/main/java/com/cozymate/cozymate_server/domain/member/converter/MemberConverter.java (1)
MemberConverter(16-117)
src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketHandshakeInterceptor.java (3)
src/main/java/com/cozymate/cozymate_server/global/websocket/StompInterceptor.java (1)
Slf4j(23-82)src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketEventListener.java (1)
Slf4j(12-29)src/main/java/com/cozymate/cozymate_server/global/websocket/repository/WebSocketSessionRepository.java (1)
Slf4j(12-112)
src/main/java/com/cozymate/cozymate_server/domain/member/service/MemberCacheService.java (3)
src/main/java/com/cozymate/cozymate_server/domain/member/converter/MemberConverter.java (1)
MemberConverter(16-117)src/main/java/com/cozymate/cozymate_server/domain/member/controller/MemberController.java (1)
RequiredArgsConstructor(40-166)src/main/java/com/cozymate/cozymate_server/domain/member/repository/MemberRepositoryService.java (1)
Service(13-68)
🔇 Additional comments (27)
config (1)
1-1: 서브모듈 커밋 갱신 확인 완료새 커밋 포인터로의 업데이트 외 추가 우려 사항 없습니다.
build.gradle (1)
95-104: LGTM!채팅 기능 구현에 필요한 의존성이 적절하게 추가되었습니다:
- WebSocket: STOMP 기반 실시간 채팅
- MongoDB: 채팅 메시지 영구 저장
- Spring Retry & Aspects: 재시도 로직 및 AOP 지원
모든 의존성은 Spring Boot BOM에 의해 버전이 관리됩니다.
src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoomMember.java (1)
28-32: LGTM!
@ManyToOne(fetch = FetchType.LAZY)설정이 적절하게 적용되어 N+1 문제를 방지합니다. 지연 로딩은 채팅방 멤버 조회 시 불필요한 연관 엔터티 로딩을 피할 수 있습니다.src/main/java/com/cozymate/cozymate_server/global/redispubsub/event/StompSubEvent.java (1)
3-5: LGTM!단순하고 명확한 이벤트 레코드 구조입니다. chatRoomId만을 전달하는 목적에 적합합니다.
src/main/java/com/cozymate/cozymate_server/global/config/JpaConfig.java (1)
12-16: 다중 트랜잭션 매니저 환경에서 명시적 지정을 검토하세요.
@PrimaryJpaTransactionManager가 기본값이므로, MongoDB 작업을 수행하는 서비스나 리포지토리에서@Transactional(transactionManager = "mongoTransactionManager")가 제대로 지정되어 있는지 직접 확인해주세요.src/main/java/com/cozymate/cozymate_server/CozymateServerApplication.java (1)
7-14: 승인 완료
@EnableRetry가 추가되었으며,ChatRoomService.enterChatRoom메서드에 적용된@Retryable(retryFor=ObjectOptimisticLockingFailureException.class, maxAttempts=10, backoff=@Backoff(delay=100, multiplier=1.5, maxDelay=1000))설정이 적절히 구성되었습니다.src/main/java/com/cozymate/cozymate_server/global/config/SecurityConfig.java (1)
98-100: WebSocket 인증/인가 처리 확인됨WebSocketHandshakeInterceptor에서 JWT 검증 및 clientId 설정을, StompInterceptor에서 CONNECT·SUBSCRIBE 시 권한 검증을 수행하고 있어
/ws/**를permitAll로 설정해도 안전합니다.src/main/java/com/cozymate/cozymate_server/global/redispubsub/event/StompDisconnectEvent.java (1)
3-5: LGTM!WebSocket 연결 해제 이벤트를 위한 간결하고 명확한 레코드 구조입니다.
src/main/java/com/cozymate/cozymate_server/domain/member/dto/MemberCachingDTO.java (1)
5-9: LGTM!멤버 캐싱을 위한 DTO 구조가 적절하며, Builder 패턴을 통해 명확한 인스턴스 생성이 가능합니다.
src/main/java/com/cozymate/cozymate_server/domain/sqs/dto/FcmSQSMessage.java (1)
15-16: LGTM!채팅방 ID 필드 추가가 기존 구조와 일관성 있게 구현되었습니다.
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/converter/EventConverter.java (1)
91-97: LGTM!CreateChatRequestDTO에서 SentChatEvent로의 변환 로직이 명확하며, 파일 내 다른 변환 메서드들과 일관된 패턴을 따르고 있습니다.
src/main/java/com/cozymate/cozymate_server/domain/chatroom/dto/response/ChatRoomResponseDTO.java (1)
5-10: LGTM!채팅방 응답 DTO 구조가 명확하며, 필요한 정보들이 적절하게 포함되어 있습니다.
src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPubSubListener.java (1)
30-32: LGTM!역직렬화된 메시지를 WebSocket으로 브로드캐스트하는 로직이 적절하며, ApiResponse로 래핑하여 일관된 응답 형식을 제공합니다.
src/main/java/com/cozymate/cozymate_server/global/config/ObjectMapperConfig.java (1)
12-19: LGTM!ObjectMapper 설정이 적절합니다. JavaTimeModule 등록과 타임스탬프 비활성화를 통해 Java 8 날짜/시간 타입의 일관된 JSON 직렬화를 보장합니다.
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/SentChatEvent.java (1)
5-10: LGTM!채팅 전송 이벤트 구조가 명확하며, Chat 엔터티의 핵심 필드들과 일관성 있게 정의되었습니다.
src/main/java/com/cozymate/cozymate_server/global/response/code/status/ErrorStatus.java (1)
225-231: 채팅 도메인 에러 추가 LGTM기존 패턴과 일관됩니다.
src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoom.java (1)
35-36: 낙관적 잠금을 통한 동시성 제어가 잘 구현되었습니다.
@Version어노테이션을 사용하여 참가자 수 업데이트 시 발생할 수 있는 동시성 문제를 효과적으로 방지하고 있습니다.src/main/java/com/cozymate/cozymate_server/domain/chat/controller/ChatController.java (1)
30-33: 입력 검증이 적절히 적용되었습니다.
@Valid어노테이션을 통해CreateChatRequestDTO의 입력 검증을 수행하는 것은 좋은 방법입니다. WebSocket 메시지에서도 유효하지 않은 데이터가 처리되지 않도록 보장합니다.src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisSubscriberManager.java (2)
28-47: 동시성 처리가 적절히 구현되었습니다.
ConcurrentHashMap의compute메서드를 사용하여 원자적으로 활성 사용자 수를 관리하고 Redis 구독을 처리하는 것은 좋은 접근입니다. 멀티스레드 환경에서 안전하게 동작합니다.
62-65: 불필요한 null 체크 제거
ConcurrentHashMap과computeIfPresent로직상topicMap.remove(chatRoomId)가 null을 반환할 수 없으므로if (Objects.isNull(channelTopic)) { … }분기는 삭제하고 곧바로
container.removeMessageListener(messageListenerAdapter, channelTopic)만 호출하세요.Likely an incorrect or invalid review comment.
src/main/java/com/cozymate/cozymate_server/global/websocket/StompInterceptor.java (2)
71-81: 채팅방 구독 권한 검증이 잘 구현되었습니다.회원이 자신의 대학교 채팅방만 구독할 수 있도록 검증하는 로직은 보안상 중요하며 올바르게 구현되어 있습니다. 적절한 예외 메시지도 제공합니다.
38-42: clientId의 null 안전성을 확인하세요.
sessionAttributes.get("clientId")가 null을 반환할 수 있습니다. WebSocketHandshakeInterceptor에서 설정하지만, 핸드셰이크를 우회하거나 실패한 경우 null일 수 있습니다.Line 39, 47, 56, 65에서
clientId를 사용하기 전에 null 체크를 추가하는 것을 고려하세요:Map<String, Object> sessionAttributes = accessor.getSessionAttributes(); String clientId = (String) sessionAttributes.get("clientId"); + + if (clientId == null) { + log.warn("[StompInterceptor] clientId가 세션에 없습니다."); + throw new MessageDeliveryException("인증되지 않은 연결"); + }src/main/java/com/cozymate/cozymate_server/domain/chatroom/controller/ChatRoomController.java (1)
31-37: 채팅방 목록 조회 구현이 깔끔합니다.인증된 회원의 대학교에 속한 채팅방만 조회하는 로직이 잘 분리되어 있으며, 표준 Spring MVC 패턴을 따릅니다.
src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketHandshakeInterceptor.java (1)
28-56: WebSocket 핸드셰이크 시 JWT 인증이 올바르게 구현되었습니다.JWT 토큰을 검증하고 유효하지 않은 경우 연결을 거부하는 로직이 적절히 구현되어 있습니다.
clientId를 세션 속성에 저장하여 이후 STOMP 인터셉터에서 사용할 수 있도록 한 것도 좋습니다.src/main/java/com/cozymate/cozymate_server/domain/chat/converter/ChatConverter.java (2)
13-58: 변환 로직이 명확하고 일관성 있게 구현되었습니다.빌더 패턴을 사용한 변환 메서드들이 읽기 쉽고 유지보수하기 좋게 구현되어 있습니다.
26-37: toChatPubDTO의 sequence 0L 고정은 의도된 동작입니다
ChatPubDTO 정의에서// pub sequence는 0 고정주석이 명시되어 있어 변경이 불필요합니다.src/main/java/com/cozymate/cozymate_server/domain/sqs/service/SQSMessageCreator.java (1)
138-151: 채팅 알림을 위한 다중 수신자 처리가 잘 구현되었습니다.채팅방의 여러 회원에게 알림을 보내기 위해
List<Member>를 받아 각 회원별로SQSMessageResult를 생성하는 접근 방식이 적절합니다.
| public record CreateChatRequestDTO( | ||
| Long memberId, | ||
| @NotBlank(message = "채팅 내용이 공백일 수 없습니다.") | ||
| @Size(max = 300, message = "300자 이하로 입력해주세요.") | ||
| String content, | ||
| Long chatRoomId | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
memberId와 chatRoomId에 null 검증을 추가하세요.
content 필드는 적절한 검증이 있지만, memberId와 chatRoomId에는 null 검증이 없습니다. 이 필드들이 필수인 경우, 다운스트림에서 NPE가 발생할 수 있습니다.
다음 diff를 적용하여 null 검증을 추가하세요:
+import jakarta.validation.constraints.NotNull;
+
public record CreateChatRequestDTO(
+ @NotNull(message = "회원 ID는 필수입니다.")
Long memberId,
@NotBlank(message = "채팅 내용이 공백일 수 없습니다.")
@Size(max = 300, message = "300자 이하로 입력해주세요.")
String content,
+ @NotNull(message = "채팅방 ID는 필수입니다.")
Long chatRoomId
) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public record CreateChatRequestDTO( | |
| Long memberId, | |
| @NotBlank(message = "채팅 내용이 공백일 수 없습니다.") | |
| @Size(max = 300, message = "300자 이하로 입력해주세요.") | |
| String content, | |
| Long chatRoomId | |
| ) { | |
| import jakarta.validation.constraints.NotNull; | |
| public record CreateChatRequestDTO( | |
| @NotNull(message = "회원 ID는 필수입니다.") | |
| Long memberId, | |
| @NotBlank(message = "채팅 내용이 공백일 수 없습니다.") | |
| @Size(max = 300, message = "300자 이하로 입력해주세요.") | |
| String content, | |
| @NotNull(message = "채팅방 ID는 필수입니다.") | |
| Long chatRoomId | |
| ) { | |
| // ... | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/cozymate/cozymate_server/domain/chat/dto/request/CreateChatRequestDTO.java
around lines 6 to 12, memberId and chatRoomId currently lack null validation
which can cause NPEs; add @NotNull to both fields (with clear message like "필수
항목입니다.") so validation fails fast, and ensure you import
javax.validation.constraints.NotNull (or jakarta.validation.constraints.NotNull)
alongside the existing annotations.
src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java
Outdated
Show resolved
Hide resolved
| String script = """ | ||
| local clientId = redis.call('HGET', KEYS[1], ARGV[1]) | ||
| if clientId then | ||
| redis.call('SADD', KEYS[2], clientId) | ||
| redis.call('HSET', KEYS[3], ARGV[1], ARGV[2]) | ||
| redis.call('HSET', KEYS[4], ARGV[1], clientId) | ||
| end | ||
| """; | ||
|
|
||
| String key1 = CONNECT_KEY; | ||
| String key2 = SUBSCRIBERS_CHATROOM_KEY_PREFIX + chatRoomId; | ||
| String key3 = SUBSCRIBE_CHATROOM_KEY; | ||
| String key4 = SUBSCRIBE_CLIENT_KEY; | ||
|
|
||
| redisTemplate.execute( | ||
| (RedisCallback<Void>) conn -> { | ||
| redisTemplate.getStringSerializer().deserialize( | ||
| conn.eval( | ||
| script.getBytes(), | ||
| ReturnType.STATUS, | ||
| 4, | ||
| key1.getBytes(), key2.getBytes(), key3.getBytes(), key4.getBytes(), | ||
| sessionId.getBytes(), chatRoomId.getBytes() | ||
| ) | ||
| ); | ||
|
|
||
| return null; | ||
| } | ||
| ); |
There was a problem hiding this comment.
세션 이동 시 기존 구독자가 정리되지 않습니다
현재 Lua 스크립트가 동일 세션으로 다른 채팅방을 다시 SUBSCRIBE할 때, 이전 채팅방의 집합에서 clientId를 제거하지 않습니다. 그래서 SUBSCRIBERS:chatRoomId:<이전방>에 clientId가 남아 있어 실제로는 구독하지 않는 방에도 메시지가 전달되는 등 잘못된 온라인 사용자 계산이 발생합니다. 이전 chatRoomId를 조회해서 다르면 SREM으로 정리하도록 스크립트를 보강해 주세요.
- String script = """
- local clientId = redis.call('HGET', KEYS[1], ARGV[1])
-
- if clientId then
- redis.call('SADD', KEYS[2], clientId)
- redis.call('HSET', KEYS[3], ARGV[1], ARGV[2])
- redis.call('HSET', KEYS[4], ARGV[1], clientId)
- end
- """;
+ String script = """
+ local clientId = redis.call('HGET', KEYS[1], ARGV[1])
+ local previousChatRoomId = redis.call('HGET', KEYS[3], ARGV[1])
+ local chatRoomId = ARGV[2]
+
+ if clientId then
+ if previousChatRoomId and previousChatRoomId ~= chatRoomId then
+ redis.call('SREM', KEYS[2] .. previousChatRoomId, clientId)
+ end
+ redis.call('SADD', KEYS[2] .. chatRoomId, clientId)
+ redis.call('HSET', KEYS[3], ARGV[1], chatRoomId)
+ redis.call('HSET', KEYS[4], ARGV[1], clientId)
+ end
+ """;
- String key2 = SUBSCRIBERS_CHATROOM_KEY_PREFIX + chatRoomId;
+ String key2 = SUBSCRIBERS_CHATROOM_KEY_PREFIX;
...ava/com/cozymate/cozymate_server/global/websocket/repository/WebSocketSessionRepository.java
Show resolved
Hide resolved
| @Override | ||
| public Message<byte[]> handleClientMessageProcessingError(Message<byte[]> clientMessage, | ||
| Throwable ex) { | ||
| StompHeaderAccessor accessor = StompHeaderAccessor.create(StompCommand.ERROR); | ||
| accessor.setLeaveMutable(true); | ||
|
|
||
| ApiResponse<Object> apiResponse = ApiResponse.onFailure( | ||
| ErrorStatus._BAD_REQUEST.getCode(), ex.getMessage(), null); | ||
|
|
||
| return errorMessage(accessor, apiResponse); | ||
| } | ||
|
|
||
| private Message<byte[]> errorMessage(StompHeaderAccessor accessor, | ||
| ApiResponse<Object> apiResponse) { | ||
| try { | ||
| String errorMessage = objectMapper.writeValueAsString(apiResponse); | ||
| return MessageBuilder.createMessage(errorMessage.getBytes(StandardCharsets.UTF_8), | ||
| accessor.getMessageHeaders()); | ||
| } catch (JsonProcessingException e) { | ||
| return MessageBuilder.createMessage(EMPTY_PAYLOAD, accessor.getMessageHeaders()); | ||
| } | ||
| } |
There was a problem hiding this comment.
STOMP ERROR 페이로드 위생 및 헤더 보강
- 클라이언트로 예외 메시지(raw ex.getMessage())를 그대로 노출하면 민감 정보가 새어 나갈 수 있습니다. WebSocketException만 상세, 그 외는 일반 메시지로 축소를 권장합니다.
- content-type 헤더를 JSON으로 명시하면 클라이언트 처리 일관성이 좋아집니다.
예시 수정:
+import org.springframework.util.MimeTypeUtils;
@@
- ApiResponse<Object> apiResponse = ApiResponse.onFailure(
- ErrorStatus._BAD_REQUEST.getCode(), ex.getMessage(), null);
+ String msg = (ex instanceof WebSocketException)
+ ? ex.getMessage()
+ : ErrorStatus._BAD_REQUEST.getMessage();
+ ApiResponse<Object> apiResponse = ApiResponse.onFailure(
+ ErrorStatus._BAD_REQUEST.getCode(), msg, null);
@@
- private Message<byte[]> errorMessage(StompHeaderAccessor accessor,
+ private Message<byte[]> errorMessage(StompHeaderAccessor accessor,
ApiResponse<Object> apiResponse) {
try {
+ accessor.setContentType(MimeTypeUtils.APPLICATION_JSON);
String errorMessage = objectMapper.writeValueAsString(apiResponse);
return MessageBuilder.createMessage(errorMessage.getBytes(StandardCharsets.UTF_8),
accessor.getMessageHeaders());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public Message<byte[]> handleClientMessageProcessingError(Message<byte[]> clientMessage, | |
| Throwable ex) { | |
| StompHeaderAccessor accessor = StompHeaderAccessor.create(StompCommand.ERROR); | |
| accessor.setLeaveMutable(true); | |
| ApiResponse<Object> apiResponse = ApiResponse.onFailure( | |
| ErrorStatus._BAD_REQUEST.getCode(), ex.getMessage(), null); | |
| return errorMessage(accessor, apiResponse); | |
| } | |
| private Message<byte[]> errorMessage(StompHeaderAccessor accessor, | |
| ApiResponse<Object> apiResponse) { | |
| try { | |
| String errorMessage = objectMapper.writeValueAsString(apiResponse); | |
| return MessageBuilder.createMessage(errorMessage.getBytes(StandardCharsets.UTF_8), | |
| accessor.getMessageHeaders()); | |
| } catch (JsonProcessingException e) { | |
| return MessageBuilder.createMessage(EMPTY_PAYLOAD, accessor.getMessageHeaders()); | |
| } | |
| } | |
| import org.springframework.util.MimeTypeUtils; | |
| @@ | |
| @Override | |
| public Message<byte[]> handleClientMessageProcessingError(Message<byte[]> clientMessage, | |
| Throwable ex) { | |
| StompHeaderAccessor accessor = StompHeaderAccessor.create(StompCommand.ERROR); | |
| accessor.setLeaveMutable(true); | |
| - | |
| - ApiResponse<Object> apiResponse = ApiResponse.onFailure( | |
| String msg = (ex instanceof WebSocketException) | |
| ? ex.getMessage() | |
| : ErrorStatus._BAD_REQUEST.getMessage(); | |
| ApiResponse<Object> apiResponse = ApiResponse.onFailure( | |
| ErrorStatus._BAD_REQUEST.getCode(), msg, null); | |
| return errorMessage(accessor, apiResponse); | |
| } | |
| @@ | |
| private Message<byte[]> errorMessage(StompHeaderAccessor accessor, | |
| ApiResponse<Object> apiResponse) { | |
| try { | |
| accessor.setContentType(MimeTypeUtils.APPLICATION_JSON); | |
| String errorMessage = objectMapper.writeValueAsString(apiResponse); | |
| return MessageBuilder.createMessage(errorMessage.getBytes(StandardCharsets.UTF_8), | |
| accessor.getMessageHeaders()); |
src/main/java/com/cozymate/cozymate_server/global/websocket/StompInterceptor.java
Show resolved
Hide resolved
| String chatRoomId = webSocketSessionRepository.deleteChatRoomSubscribingMembers(sessionId); | ||
| webSocketSessionRepository.deleteSessionAndClientId(sessionId); | ||
|
|
||
| eventPublisher.publishEvent(new StompDisconnectEvent(Long.parseLong(chatRoomId))); | ||
| log.info("[WebSocketEventListener] 세션 종료 처리: {}", sessionId); |
There was a problem hiding this comment.
세션 미구독 시 NPE 발생 가능
Line 23에서 deleteChatRoomSubscribingMembers가 null을 반환할 수 있는데, Line 26에서 즉시 Long.parseLong(chatRoomId)를 호출하면 세션이 구독을 완료하지 못한 상태로 끊길 때 NullPointerException이 발생합니다. 연결 직후 곧바로 끊거나 다른 토픽만 구독한 세션의 정리 시 서비스 전체가 깨집니다. chatRoomId 존재 여부와 숫자 여부를 확인한 뒤 이벤트를 발행하도록 방어 로직을 추가해주세요.
- String chatRoomId = webSocketSessionRepository.deleteChatRoomSubscribingMembers(sessionId);
+ String chatRoomId = webSocketSessionRepository.deleteChatRoomSubscribingMembers(sessionId);
webSocketSessionRepository.deleteSessionAndClientId(sessionId);
- eventPublisher.publishEvent(new StompDisconnectEvent(Long.parseLong(chatRoomId)));
- log.info("[WebSocketEventListener] 세션 종료 처리: {}", sessionId);
+ if (chatRoomId != null && !chatRoomId.isBlank()) {
+ try {
+ eventPublisher.publishEvent(new StompDisconnectEvent(Long.parseLong(chatRoomId)));
+ } catch (NumberFormatException ex) {
+ log.warn("[WebSocketEventListener] 숫자가 아닌 chatRoomId가 반환되었습니다. sessionId={}, chatRoomId={}", sessionId, chatRoomId, ex);
+ }
+ }
+ log.info("[WebSocketEventListener] 세션 종료 처리: {}", sessionId);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String chatRoomId = webSocketSessionRepository.deleteChatRoomSubscribingMembers(sessionId); | |
| webSocketSessionRepository.deleteSessionAndClientId(sessionId); | |
| eventPublisher.publishEvent(new StompDisconnectEvent(Long.parseLong(chatRoomId))); | |
| log.info("[WebSocketEventListener] 세션 종료 처리: {}", sessionId); | |
| String chatRoomId = webSocketSessionRepository.deleteChatRoomSubscribingMembers(sessionId); | |
| webSocketSessionRepository.deleteSessionAndClientId(sessionId); | |
| if (chatRoomId != null && !chatRoomId.isBlank()) { | |
| try { | |
| eventPublisher.publishEvent(new StompDisconnectEvent(Long.parseLong(chatRoomId))); | |
| } catch (NumberFormatException ex) { | |
| log.warn( | |
| "[WebSocketEventListener] 숫자가 아닌 chatRoomId가 반환되었습니다. sessionId={}, chatRoomId={}", | |
| sessionId, chatRoomId, ex | |
| ); | |
| } | |
| } | |
| log.info("[WebSocketEventListener] 세션 종료 처리: {}", sessionId); |
🤖 Prompt for AI Agents
In
src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketEventListener.java
around lines 23-27, the returned chatRoomId from
deleteChatRoomSubscribingMembers can be null or non-numeric causing
NPE/NumberFormatException when Long.parseLong is called; add a defensive check:
after calling deleteChatRoomSubscribingMembers and deleteSessionAndClientId,
verify chatRoomId is not null/empty and matches a numeric pattern (or catch
NumberFormatException) before calling eventPublisher.publishEvent(new
StompDisconnectEvent(...)); if invalid, skip publishing and log a debug/info
message indicating no chat-room event is emitted for that session.
| @Slf4j | ||
| @Repository | ||
| @RequiredArgsConstructor | ||
| public class WebSocketSessionRepository { |
There was a problem hiding this comment.
웹소켓 연결 관련한 데이터를 redis에 저장, 삭제하는 클래스입니다
| String authHeader = httpServletRequest.getHeader(JwtUtil.HEADER_ATTRIBUTE_NAME_AUTHORIZATION); | ||
|
|
||
| if (Objects.nonNull(authHeader) && authHeader.startsWith(JwtUtil.TOKEN_PREFIX)) { | ||
| String jwt = authHeader.substring(JwtUtil.TOKEN_PREFIX.length()); | ||
| try { | ||
| jwtUtil.validateToken(jwt); | ||
| } catch (Exception e) { | ||
| log.info("WebSocketHandShake 전 jwt 인증 실패"); | ||
| response.setStatusCode(HttpStatus.UNAUTHORIZED); | ||
| return false; | ||
| } | ||
|
|
||
| String clientId = jwtUtil.extractUserName(jwt); | ||
| attributes.put("clientId", clientId); // StompInterceptor에서 꺼내어 사용 예정 | ||
| } else { | ||
| log.info("WebSocketHandShake 전 헤더에 jwt 없음"); | ||
| response.setStatusCode(HttpStatus.UNAUTHORIZED); | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
http -> ws 업그레이드 시 jwt로 검증하고 있습니다.
여기서 jwt에서 꺼낸 clientId 넘겨서 stomp CONNECT부터 DISCONNECT까지 redis에 세션id랑 clientId랑 매핑해서 사용
| private void validateCanSubscribe(String clientId, String chatRoomId) { | ||
| Member member = memberRepository.findByClientId(clientId) | ||
| .orElseThrow(() -> new MessageDeliveryException("토픽 Sub중, member not found")); | ||
|
|
||
| ChatRoom chatRoom = chatRoomRepository.findById(Long.parseLong(chatRoomId)) | ||
| .orElseThrow(() -> new MessageDeliveryException("sub하려는 채팅방이 존재하지 않음")); | ||
|
|
||
| if (!member.getUniversity().getId().equals(chatRoom.getUniversity().getId())) { | ||
| throw new MessageDeliveryException("다른 대학의 채팅방 sub 불가"); | ||
| } | ||
| } |
There was a problem hiding this comment.
여기서 발생하는 예외는 StompErrorHandler에서 잡아서 ERROR 프레임을 클라이언트에 전송합니다
| // 클라이언트는 "/user/queue/errors" 경로를 필수 구독 | ||
| @MessageExceptionHandler | ||
| @SendToUser(destinations = "/queue/errors") | ||
| public ApiResponse<Void> webSocketException(WebSocketException e) { | ||
| return ApiResponse.onFailure(e.getCode().getReasonHttpStatus().getCode(), | ||
| e.getCode().getMessage(), null); | ||
| } | ||
|
|
||
| // 클라이언트는 "/user/queue/errors" 경로를 필수 구독 | ||
| @MessageExceptionHandler | ||
| @SendToUser(destinations = "/queue/errors") | ||
| public ApiResponse<Map<String, String>> handleValidationException( | ||
| org.springframework.messaging.handler.annotation.support.MethodArgumentNotValidException e) { |
There was a problem hiding this comment.
Stomp SEND 프레임으로 전송되어 @MessageMapping(@ReqeustMapping과 유사)에서 발생하는 예외에 대해 잡아주는 부분입니다
| @Slf4j | ||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class RedisSubscriberManager { |
There was a problem hiding this comment.
서버에서 Redis의 Topic 구독을 관리하는 클래스입니다.
해당 서버에서 한명이라도 채팅방에 접속해있다면 서버는 chatroom:{채팅방id} topic을 구독, 0명이라면 구독 x
| import org.springframework.orm.jpa.JpaTransactionManager; | ||
|
|
||
| @Configuration | ||
| public class JpaConfig { |
There was a problem hiding this comment.
Mongo트랜잭션매니저 빈 등록하면서 기존 @transactional이 트랜잭션매니저를 못찾아서 @primary로 등록해주었습니다
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class MemberCacheService { |
There was a problem hiding this comment.
사용자의 nickname과 persona를 캐싱해서 사용하도록 했습니다. TTL 1시간으로 잡아뒀어요
There was a problem hiding this comment.
사용자 정보를 캐싱하는 건 채팅방 때문인거 같은데, dto하나가 만료된 상황에 여러명이 동시에 조회하면 다들 miss나는 건가요? 추후에 개선하면 좋을거 같아요. 한 번 미스나서 배치하는 동안에는 락거는 방향으로
| @Transactional | ||
| @Retryable( | ||
| retryFor = ObjectOptimisticLockingFailureException.class, | ||
| maxAttempts = 10, | ||
| backoff = @Backoff(delay = 100, multiplier = 1.5, maxDelay = 1000) | ||
| ) |
There was a problem hiding this comment.
채팅방에 참여 인원 요구 사항이 한번이라도 거쳐간 사람에 대해서만 단 한번 +1을 하는 구조라서, 최초의 채팅방 입장때만 1증가해주면 되고, 충돌이 잘 안일어 날 것이라 생각해서 낙관적 락을 걸고, 충돌 시 재시도 하도록 했습니다
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (1)
357-366: 🔴 발신자 본인에게 푸시 알림이 발송되는 중대 결함이전 리뷰에서 지적된 문제가 여전히 수정되지 않았습니다. 현재
notSubscribing리스트에 발신자(sender)가 포함되어 있어, 본인이 보낸 메시지에 대해 본인에게 푸시 알림이 발송됩니다.아래 diff를 적용하여 발신자를 제외한 대상에게만 푸시를 발송하도록 수정해주세요:
List<Member> notSubscribing = chatRoomMembers.stream() .map(ChatRoomMember::getMember) .filter(m -> !subscribingMembers.contains(m.getClientId())) .toList(); Member sender = memberRepositoryService.getMemberByIdOrThrow(sentChatEvent.memberId()); + + // 발신자는 푸시 대상에서 제외 + List<Member> targets = notSubscribing.stream() + .filter(m -> !m.getId().equals(sender.getId())) + .toList(); List<SQSMessageResult> results = sqsMessageCreator.createWithChatRoomId( - sender, notSubscribing, sentChatEvent.content(), sentChatEvent.chatRoomId(), + sender, targets, sentChatEvent.content(), sentChatEvent.chatRoomId(), NotificationType.ARRIVE_CHAT);
🧹 Nitpick comments (2)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (2)
372-377: 주석을 메서드 상단으로 이동하거나 한글로 작성 권장테스트 결과 및 배치 크기 산정 근거를 담은 주석이 로직 중간에 위치해 있습니다. 이러한 설명은 메서드 JavaDoc이나 메서드 시작 부분에 위치하는 것이 가독성에 더 좋습니다. 또한 코드베이스의 다른 주석들과 일관성을 위해 한글로 작성하는 것을 고려해주세요.
379-393: 배치 처리 로직을 헬퍼 메서드로 추출 권장배치 분할 로직이 다소 복잡하여 가독성을 저해하고 있습니다. 재사용성과 테스트 용이성을 고려하여 별도의 private 메서드로 추출하는 것을 권장합니다.
예시:
private <T> List<List<T>> partitionList(List<T> list, int batchSize) { List<List<T>> batches = new ArrayList<>(); for (int i = 0; i < list.size(); i += batchSize) { int end = Math.min(i + batchSize, list.size()); batches.add(list.subList(i, end)); } return batches; }그런 다음 호출 부분을 간소화:
- if (fcmSqsMessageList.size() > BATCH_SIZE) { - List<List<FcmSQSMessage>> batchList = new ArrayList<>(); - for (int start = 0; start < fcmSqsMessageList.size(); start += BATCH_SIZE) { - int end = start + BATCH_SIZE; - - if (end > fcmSqsMessageList.size()) { - end = fcmSqsMessageList.size(); - } - - batchList.add(fcmSqsMessageList.subList(start, end)); - } - - batchList.forEach( - fcmSQSMessages -> sqsMessageSender.sendMessage(fcmSQSMessages) - ); + if (fcmSqsMessageList.size() > BATCH_SIZE) { + List<List<FcmSQSMessage>> batches = partitionList(fcmSqsMessageList, BATCH_SIZE); + batches.forEach(sqsMessageSender::sendMessage); } else { sqsMessageSender.sendMessage(fcmSqsMessageList); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java(6 hunks)
🔇 Additional comments (1)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (1)
345-346: @eventlistener 사용이 적절합니다
ChatService.sendChat에@Transactional이 없어@TransactionalEventListener로는 이벤트가 실행되지 않습니다.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (1)
356-366: 발신자 제외 필터 추가 필요현재
notSubscribing리스트에 발신자가 포함되어 있어 자기 자신에게 푸시가 발송될 수 있습니다. 이전 리뷰에서 이미 지적된 사항으로, 아래와 같이 발신자를 제외한targets를 생성해 사용해 주세요.List<Member> notSubscribing = chatRoomMembers.stream() .map(ChatRoomMember::getMember) .filter(m -> !subscribingMembers.contains(m.getClientId())) .toList(); Member sender = memberRepositoryService.getMemberByIdOrThrow(sentChatEvent.memberId()); +List<Member> targets = notSubscribing.stream() + .filter(m -> !m.getId().equals(sender.getId())) + .toList(); -List<SQSMessageResult> results = sqsMessageCreator.createWithChatRoomId( - sender, notSubscribing, sentChatEvent.content(), sentChatEvent.chatRoomId(), +List<SQSMessageResult> results = sqsMessageCreator.createWithChatRoomId( + sender, targets, sentChatEvent.content(), sentChatEvent.chatRoomId(), NotificationType.ARRIVE_CHAT);
🧹 Nitpick comments (1)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (1)
377-387: 배치 전송 로직 확인 완료 및 로깅 개선 제안배치 전송 로직은 올바르게 구현되어 있습니다. 빈 리스트, 크기 초과, 일반 케이스를 모두 적절히 처리하고 있습니다.
이전 리뷰에서 언급된 ObjectMapper 직렬화 관련 이슈는 현재 코드에 해당 로직이 없어 적용되지 않습니다.
다만, 배치 전송 시 디버깅을 위한 로깅을 추가하면 운영 중 문제 추적에 도움이 될 수 있습니다.
if (!fcmSqsMessageList.isEmpty()) { if (fcmSqsMessageList.size() > BATCH_SIZE) { + log.info("채팅 알림 배치 전송 시작: 총 {} 개 메시지, {} 개 배치", + fcmSqsMessageList.size(), (fcmSqsMessageList.size() + BATCH_SIZE - 1) / BATCH_SIZE); for (int start = 0; start < fcmSqsMessageList.size(); start += BATCH_SIZE) { int end = Math.min(start + BATCH_SIZE, fcmSqsMessageList.size()); sqsMessageSender.sendMessage(fcmSqsMessageList.subList(start, end)); + log.debug("배치 전송 완료: {}-{}", start, end - 1); } } else { sqsMessageSender.sendMessage(fcmSqsMessageList); + log.debug("채팅 알림 전송 완료: {} 개 메시지", fcmSqsMessageList.size()); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java(6 hunks)
🔇 Additional comments (3)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (3)
3-7: LGTM!새로운 채팅 알림 흐름에 필요한 의존성이 올바르게 추가되었습니다.
Also applies to: 52-54
344-349: @eventlistener 사용 의도 확인 필요다른 알림 핸들러들은
@TransactionalEventListener를 사용하지만, 이 메서드는@EventListener를 사용합니다. 이는 트랜잭션 커밋 후가 아닌 이벤트 발행 즉시 실행됩니다.Redis Streams 기반 채팅 아키텍처에서는 의도된 동작일 수 있으나, 채팅 메시지가 아직 영속화되지 않은 상태에서 알림이 발송될 가능성을 검토해 주세요.
367-369: 알림 로그 영속화 누락 확인 필요다른 알림 핸들러들과 달리 이 메서드는
NotificationLog를 저장하지 않습니다.SQSMessageResult에notificationLog()가 포함되어 있으나 이를 영속화하는 로직이 없습니다.채팅 알림의 높은 발생 빈도를 고려한 의도적인 설계인지 확인해 주세요. 만약 의도적이지 않다면, 아래와 같이 로그 저장 로직을 추가하는 것을 권장합니다.
List<FcmSQSMessage> fcmSqsMessageList = results.stream() .flatMap(r -> r.fcmSQSMessageList().stream()) .toList(); +// NotificationLog 저장 +results.forEach(r -> + notificationLogRepositoryService.createNotificationLog(r.notificationLog()));
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketEventListener.java (1)
28-31: Long.parseLong 호출 시 NumberFormatException 방어 처리 고려현재는
StompInterceptor의validateCanSubscribe에서 chatRoomId 형식을 검증하므로 실제로는 문제가 없지만, 방어적 프로그래밍 차원에서NumberFormatException처리를 추가하는 것을 권장합니다.다음과 같이 try-catch를 추가할 수 있습니다:
if (Objects.nonNull(chatRoomId)) { - eventPublisher.publishEvent(new StompDisconnectEvent(Long.parseLong(chatRoomId))); + try { + eventPublisher.publishEvent(new StompDisconnectEvent(Long.parseLong(chatRoomId))); + } catch (NumberFormatException ex) { + log.warn("[WebSocketEventListener] chatRoomId 파싱 실패: sessionId={}, chatRoomId={}", sessionId, chatRoomId, ex); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/cozymate/cozymate_server/global/websocket/StompInterceptor.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketEventListener.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/cozymate/cozymate_server/global/websocket/StompInterceptor.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketEventListener.java (2)
src/main/java/com/cozymate/cozymate_server/global/websocket/StompInterceptor.java (1)
Slf4j(23-81)src/main/java/com/cozymate/cozymate_server/global/websocket/repository/WebSocketSessionRepository.java (1)
Slf4j(12-112)
🔇 Additional comments (2)
src/main/java/com/cozymate/cozymate_server/global/websocket/WebSocketEventListener.java (2)
1-16: 클래스 구조 및 의존성 주입이 적절합니다.Spring의 이벤트 리스너 패턴과 Lombok을 활용한 의존성 주입이 명확하고 간결하게 구현되어 있습니다.
28-31: 이전 리뷰의 NPE 문제가 정확히 해결되었습니다.
Objects.nonNull(chatRoomId)체크를 통해 SUBSCRIBE 프레임 검증 실패 시 chatRoomId가 null인 경우를 안전하게 처리하고 있습니다. 주석으로 null이 발생하는 상황도 명확히 설명되어 있어 좋습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java (3)
144-144: 로그 메시지의 오타를 수정하세요."CousumerGroup"을 "ConsumerGroup"으로 수정해야 합니다.
- log.info("CousumerGroup : {}, ack 성공 수 : {}", consumerGroupName, acknowledge); + log.info("ConsumerGroup : {}, ack 성공 수 : {}", consumerGroupName, acknowledge);
154-154: 로그 메시지의 오타를 수정하세요."CousumerGroup"을 "ConsumerGroup"으로 수정해야 합니다.
- log.info("CousumerGroup : {}, ack 성공 수 : {}", consumerGroupName, acknowledge); + log.info("ConsumerGroup : {}, ack 성공 수 : {}", consumerGroupName, acknowledge);
259-274: Redis 커넥션 리소스 누수를 수정하세요.
getConnection()으로 얻은 커넥션을 닫지 않아 리소스 누수가 발생합니다.trimStreamBeforeOldestPendingMessage메서드에서는 try-with-resources를 올바르게 사용하고 있으나, 여기서는 누락되었습니다.다음과 같이 try-with-resources로 감싸주세요:
// Stream 존재 하지 않으면, MKSTREAM 옵션을 통해 만들고, ConsumerGroup 또한 생성한다 if (!redisTemplate.hasKey(streamKey)) { - RedisAsyncCommands commands = (RedisAsyncCommands) redisTemplate - .getConnectionFactory() - .getConnection() - .getNativeConnection(); - - CommandArgs<String, String> args = new CommandArgs<>(StringCodec.UTF8) - .add(CommandKeyword.CREATE) - .add(streamKey) - .add(consumerGroupName) - .add("0") - .add("MKSTREAM"); - - commands.dispatch(CommandType.XGROUP, new StatusOutput(StringCodec.UTF8), args); + try (RedisConnection connection = redisTemplate.getConnectionFactory().getConnection()) { + @SuppressWarnings("unchecked") + RedisAsyncCommands<String, String> commands = + (RedisAsyncCommands<String, String>) connection.getNativeConnection(); + + CommandArgs<String, String> args = new CommandArgs<>(StringCodec.UTF8) + .add(CommandKeyword.CREATE) + .add(streamKey) + .add(consumerGroupName) + .add("0") + .add("MKSTREAM"); + + commands.dispatch(CommandType.XGROUP, new StatusOutput<>(StringCodec.UTF8), args); + } } else { // Stream 존재 시
🧹 Nitpick comments (5)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java (5)
112-129: 매직 넘버를 상수로 추출하세요.메서드명과 코드에 하드코딩된
21은 매직 넘버입니다. 향후 값 변경 시 메서드명도 함께 바꿔야 하는 불편함이 있습니다.다음과 같이 개선할 수 있습니다:
+ private static final int INITIAL_CHAT_LOAD_SIZE = 21; + /** - * Stream에서 가장 최신 21개 채팅 데이터 조회 (방 입장 시 조회용) + * Stream에서 가장 최신 N개 채팅 데이터 조회 (방 입장 시 조회용) */ - public List<Chat> getRecent21ChatList(Long chatRoomId) { + public List<Chat> getRecentChatList(Long chatRoomId) { List<MapRecord<String, Object, Object>> records = redisTemplate.opsForStream() .reverseRange(generateChatroomStreamKey(chatRoomId), Range.unbounded(), - Limit.limit().count(21)); + Limit.limit().count(INITIAL_CHAT_LOAD_SIZE)); log.info("redis에서 조회된 방 입장시 데이터 수 : {}", records.size());Note:
ChatService.java에서 이 메서드를 호출하는 부분도 함께 업데이트해야 합니다.
160-174: SCAN 카운트를 상수로 추출하세요.하드코딩된
50은 매직 넘버입니다. 향후 조정이 필요할 수 있으므로 상수로 관리하는 것이 좋습니다.+ private static final int STREAM_SCAN_COUNT = 50; + public List<String> getAllChatRoomStreamKeyList() { - ScanOptions options = ScanOptions.scanOptions().match("STREAM:chatroom:*").count(50) + ScanOptions options = ScanOptions.scanOptions().match("STREAM:chatroom:*").count(STREAM_SCAN_COUNT) .build();
185-188: 트림 보존 개수를 상수로 추출하세요.펜딩 메시지가 없을 때 남기는
50개는 매직 넘버입니다. 클래스 상단에 상수로 선언하면 가독성과 유지보수성이 향상됩니다.+ private static final int DEFAULT_TRIM_RETAIN_COUNT = 50; + // 펜딩 메시지가 없는 경우, 최신 50개만 남겨둔다 if (pendingSummary.getTotalPendingMessages() == 0) { - redisTemplate.opsForStream().trim(streamKey, 50); + redisTemplate.opsForStream().trim(streamKey, DEFAULT_TRIM_RETAIN_COUNT); return; }
241-252: 설정 값들을 외부화하거나 상수로 추출하세요.배치 크기(
10), 스레드 풀 크기(2), 폴링 타임아웃(20ms)이 하드코딩되어 있습니다. 운영 환경에 따라 조정이 필요할 수 있으므로application.yml에서 주입받거나 최소한 클래스 상수로 관리하는 것이 좋습니다.+ @Value("${redis.stream.batch-size:10}") + private int streamBatchSize; + + @Value("${redis.stream.thread-pool-size:2}") + private int streamThreadPoolSize; + + @Value("${redis.stream.poll-timeout-ms:20}") + private int streamPollTimeoutMs; + public StreamMessageListenerContainer createStreamMessageListenerContainer() { return StreamMessageListenerContainer.create( redisTemplate.getConnectionFactory(), StreamMessageListenerContainer .StreamMessageListenerContainerOptions.builder() - .batchSize(10) + .batchSize(streamBatchSize) - .executor(Executors.newFixedThreadPool(2)) + .executor(Executors.newFixedThreadPool(streamThreadPoolSize)) .hashKeySerializer(new StringRedisSerializer()) .hashValueSerializer(new StringRedisSerializer()) - .pollTimeout(Duration.ofMillis(20)) + .pollTimeout(Duration.ofMillis(streamPollTimeoutMs)) .build() ); }
285-297: Stream API를 사용하여 간결하게 작성하세요.수동 반복문 대신 Stream API를 사용하면 코드가 더 간결하고 읽기 쉬워집니다.
private boolean existStreamConsumerGroup(String streamKey, String consumerGroupName) { - Iterator<XInfoGroup> iterator = redisTemplate.opsForStream().groups(streamKey).stream() - .iterator(); - - while (iterator.hasNext()) { - StreamInfo.XInfoGroup xInfoGroup = iterator.next(); - if (xInfoGroup.groupName().equals(consumerGroupName)) { - return true; - } - } - - return false; + return redisTemplate.opsForStream().groups(streamKey).stream() + .anyMatch(group -> group.groupName().equals(consumerGroupName)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java (4)
src/main/java/com/cozymate/cozymate_server/domain/chat/converter/ChatConverter.java (1)
ChatConverter(13-58)src/main/java/com/cozymate/cozymate_server/global/scheduler/RedisStreamScheduler.java (1)
Slf4j(16-91)src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java (1)
Slf4j(38-221)src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamConsumer.java (1)
Slf4j(26-106)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cozymate/cozymate_server/domain/chat/service/redis/ChatStreamService.java
Outdated
Show resolved
Hide resolved
| private int participantNum; | ||
|
|
||
| @Version | ||
| private long version; |
There was a problem hiding this comment.
채팅방 참여 인원 반정규화 + 낙관적 락으로 설계했습니다.
Walkthrough실시간 채팅 기능을 위한 인프라·도메인·서비스를 대규모로 추가합니다. STOMP/WebSocket 엔드포인트와 핸드셰이크·인터셉터·에러핸들러, WebSocket 세션/구독을 Redis로 관리하는 저장소와 구독 관리자, Redis Pub/Sub 퍼블리셔·리스너가 도입되었습니다. 채팅 메시지는 MongoDB 문서로 저장되며 채팅방과 참가자는 JPA 엔티티 및 리포지토리로 관리됩니다. 멤버 캐시(Redis)·FCM/SQS 알림 생성 흐름·Spring Retry 설정(@EnableRetry)과 관련 구성(ObjectMapper, MongoTransactionManager, JPA 트랜잭션 등)도 추가되었습니다. Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as WS Client
participant Hand as WebSocketHandshakeInterceptor
participant WS as STOMP /ws
participant SI as StompInterceptor
participant RepoS as WebSocketSessionRepository
participant CC as ChatController
participant CS as ChatService
participant ChatRepo as ChatRepositoryService
participant MCache as MemberCacheService
participant RedisPub as RedisPublisher
participant Evt as ApplicationEventPublisher
rect rgba(220,240,255,0.12)
Client->>Hand: HTTP Upgrade /ws (Authorization)
Hand->>Client: validate JWT, set clientId
Client->>WS: CONNECT / SUBSCRIBE / SEND
WS->>SI: inbound message
SI->>RepoS: saveSessionAndClientId / saveChatRoomSubscribingMembers
SI-->>Evt: publish StompSubEvent (chatRoomId)
WS->>CC: @MessageMapping("/chats") SEND(dto)
CC->>CS: sendChat(dto)
CS->>ChatRepo: saveChat(document)
CS->>MCache: findMemberCachingDTO(memberId)
alt cache miss
MCache->>CS: fetch members & save to cache
end
CS->>RedisPub: publishToChat(ChatPubDTO)
CS-->>Evt: publish SentChatEvent
end
sequenceDiagram
autonumber
participant Redis as Redis Channel "chatroom:{id}"
participant L as RedisPubSubListener
participant SMT as SimpMessagingTemplate
participant Sub as Subscribed Clients
Redis-->>L: message (ChatPubDTO JSON)
L->>L: deserialize(ObjectMapper)
L->>SMT: convertAndSend("/topic/chatrooms/{id}", ApiResponse)
SMT-->>Sub: STOMP MESSAGE (payload)
sequenceDiagram
autonumber
participant Evt as ApplicationEventPublisher
participant NEL as NotificationEventListener
participant Repo as WebSocketSessionRepository
participant CRM as ChatRoomMemberRepository
participant MRS as MemberRepositoryService
participant SQC as SQSMessageCreator
participant SQS as SQS
Evt-->>NEL: SentChatEvent(chatRoomId, senderId, content)
NEL->>Repo: getSubscribingMembersInChatRoom(chatRoomId)
NEL->>CRM: findFetchMemberByChatRoomId(chatRoomId) (opt-in)
NEL->>MRS: getMemberByIdOrSocketThrow(senderId)
NEL->>SQC: createWithChatRoomId(sender, recipients, content, chatRoomId, ARRIVE_CHAT)
loop batched send
SQC-->>NEL: List<SQSMessageResult>
NEL->>SQS: send(batch)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (1)
355-364: 발신자 제외 필터 누락현재 notSubscribing 컬렉션에 발신자가 그대로 포함되어 자기 자신에게 푸시가 전달됩니다. 지난 리뷰 지적과 동일한 버그이므로 sender를 조회한 뒤 명시적으로 제외해 주세요.
- List<Member> notSubscribing = chatRoomMembers.stream() - .map(ChatRoomMember::getMember) - .filter(m -> !subscribingMembers.contains(m.getClientId())) - .toList(); - - Member sender = memberRepositoryService.getMemberByIdOrThrow(sentChatEvent.memberId()); + Member sender = memberRepositoryService.getMemberByIdOrThrow(sentChatEvent.memberId()); + + List<Member> notSubscribing = chatRoomMembers.stream() + .map(ChatRoomMember::getMember) + .filter(m -> !subscribingMembers.contains(m.getClientId())) + .filter(m -> !m.getId().equals(sender.getId())) + .toList();src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (1)
10-22: Mongo 역직렬화를 위한 기본 생성자 필요.이전 리뷰에서 지적된 것처럼, MongoDB가 문서를 역직렬화할 때 문제를 방지하려면
@NoArgsConstructor(access = AccessLevel.PROTECTED)를 추가해야 합니다.
🧹 Nitpick comments (4)
src/main/java/com/cozymate/cozymate_server/domain/chat/converter/ChatConverter.java (2)
12-12: 유틸리티 클래스 패턴 적용 권장.정적 메서드만 포함된 유틸리티 클래스는
final로 선언하고 private 생성자를 추가하여 인스턴스화를 방지하는 것이 좋습니다.다음과 같이 수정하세요:
-public class ChatConverter { +public final class ChatConverter { + + private ChatConverter() { + throw new UnsupportedOperationException("Utility class"); + }
14-21: 박싱/언박싱 제거 및 타임존 일관성 확보
- chatRoomId는 이미 Long 타입이므로
Long.valueOf(createChatRequestDTO.chatRoomId())대신createChatRequestDTO.chatRoomId()를 직접 사용하세요.LocalDateTime.now()는 시스템 기본 타임존을 사용하므로, UTC 기준 시각 또는 주입된Clock/OffsetDateTime사용을 고려하세요.src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java (1)
44-63: 트랜잭션 범위 내 외부 DB 조회 확인 필요.Line 56에서
getMemberCachingDTO를 호출하는데, 이 메서드는 캐시 미스 시 멤버 DB(아마도 JPA)를 조회합니다. MongoDB 트랜잭션 내에서 다른 데이터 소스를 조회하는 것은 성능 문제를 일으킬 수 있습니다.멤버 정보 조회를 트랜잭션 범위 밖으로 이동하거나, MongoDB에 채팅을 저장하기 전에 미리 조회하는 것을 고려하세요.
다음과 같이 리팩토링하는 것을 고려하세요:
@Transactional(transactionManager = "mongoTransactionManager") public void sendChat(CreateChatRequestDTO createChatRequestDTO) { if (!chatRoomRepositoryService.existsChatRoomMemberByChatRoomIdAndMemberId( createChatRequestDTO.chatRoomId(), createChatRequestDTO.memberId())) { throw new WebSocketException(ErrorStatus._CHATROOMMEMBER_NOT_FOUND); } + // 트랜잭션 전에 멤버 캐싱 정보 조회 + MemberCachingDTO memberCachingDTO = getMemberCachingDTO(createChatRequestDTO); + // 채팅 저장 Chat chat = ChatConverter.toDocument(createChatRequestDTO); chatRepositoryService.saveChat(chat); - // sender의 nickname, persona 조회 - MemberCachingDTO memberCachingDTO = getMemberCachingDTO(createChatRequestDTO); - // 트랜잭션 커밋 후 redis topic에 pub eventPublisher.publishEvent(ChatConverter.toChatPubDTO(chat, memberCachingDTO)); // 푸시 알림 eventPublisher.publishEvent(EventConverter.toSentChatEvent(createChatRequestDTO)); }src/main/java/com/cozymate/cozymate_server/domain/chat/controller/ChatController.java (1)
36-39: GET 메서드에 @RequestBody 사용 재검토GET 메서드에 @RequestBody를 사용하는 것은 HTTP 표준에 부합하지 않아 일부 클라이언트나 프록시가 본문을 무시할 수 있습니다. 문서화용이라면 POST로 변경하거나 Swagger에서 해당 엔드포인트를 제외하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/controller/ChatController.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/converter/ChatConverter.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/dto/redis/ChatPubDTO.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/dto/response/ChatResponseDTO.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/repository/ChatRepository.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/repository/ChatRepositoryService.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java(6 hunks)src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPublisher.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/cozymate/cozymate_server/domain/chat/dto/redis/ChatPubDTO.java
- src/main/java/com/cozymate/cozymate_server/domain/chat/repository/ChatRepositoryService.java
- src/main/java/com/cozymate/cozymate_server/domain/chat/repository/ChatRepository.java
- src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPublisher.java
🧰 Additional context used
🧬 Code graph analysis (5)
src/main/java/com/cozymate/cozymate_server/domain/chat/dto/response/ChatResponseDTO.java (1)
src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (1)
Builder(10-22)
src/main/java/com/cozymate/cozymate_server/domain/chat/Chat.java (3)
src/main/java/com/cozymate/cozymate_server/domain/chatroom/ChatRoom.java (1)
Getter(18-41)src/main/java/com/cozymate/cozymate_server/domain/member/Member.java (1)
Getter(35-103)src/main/java/com/cozymate/cozymate_server/global/utils/BaseTimeEntity.java (1)
Getter(12-29)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java (4)
src/main/java/com/cozymate/cozymate_server/domain/chat/converter/ChatConverter.java (1)
ChatConverter(12-53)src/main/java/com/cozymate/cozymate_server/domain/fcm/event/converter/EventConverter.java (1)
EventConverter(18-98)src/main/java/com/cozymate/cozymate_server/domain/member/converter/MemberConverter.java (1)
MemberConverter(16-117)src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (1)
Slf4j(42-388)
src/main/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java (3)
src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java (1)
Slf4j(31-137)src/main/java/com/cozymate/cozymate_server/domain/sqs/service/SQSMessageSender.java (1)
Slf4j(14-42)src/main/java/com/cozymate/cozymate_server/domain/sqs/service/SQSMessageCreator.java (1)
Component(18-306)
src/main/java/com/cozymate/cozymate_server/domain/chat/controller/ChatController.java (2)
src/main/java/com/cozymate/cozymate_server/domain/member/controller/MemberController.java (1)
RequiredArgsConstructor(40-166)src/main/java/com/cozymate/cozymate_server/domain/chatroom/controller/ChatRoomController.java (1)
RestController(23-58)
🔇 Additional comments (11)
src/main/java/com/cozymate/cozymate_server/domain/chat/dto/response/ChatResponseDTO.java (1)
6-17: LGTM!레코드 정의가 명확하고, @builder 어노테이션이 적절하게 적용되어 있습니다. 모든 필드가 채팅 응답에 필요한 정보를 포함하고 있습니다.
src/main/java/com/cozymate/cozymate_server/domain/chat/controller/ChatController.java (2)
30-33: LGTM!WebSocket 메시지 핸들러가 적절하게 구현되어 있으며, 요청 DTO의 유효성 검증도 포함되어 있습니다.
42-52: LGTM!이전 채팅 조회 엔드포인트가 적절하게 구현되어 있으며, 날짜 형식 검증과 인증 처리가 올바르게 적용되어 있습니다.
src/main/java/com/cozymate/cozymate_server/domain/chat/converter/ChatConverter.java (3)
23-32: LGTM!Chat 엔티티와 멤버 캐싱 DTO를 ChatPubDTO로 변환하는 로직이 명확하고 정확합니다.
34-44: LGTM!Chat 엔티티를 ChatResponseDTO로 변환하는 로직이 적절하게 구현되어 있습니다.
46-52: LGTM!ChatListResponseDTO 생성 로직이 간결하고 명확합니다.
src/main/java/com/cozymate/cozymate_server/domain/chat/service/ChatService.java (5)
65-73: LGTM!최근 채팅 목록 조회 로직이 간결하고 명확하게 구현되어 있습니다.
75-86: LGTM!이전 채팅 조회 로직이 적절하게 구현되어 있으며, 채팅방 멤버십 검증도 포함되어 있습니다.
88-103: LGTM!멤버 캐싱 조회 로직이 표준적인 look-aside 캐시 패턴으로 잘 구현되어 있습니다.
105-108: LGTM!페이지네이션 요청 생성 로직이 정확하며,
PAGE_SIZE + 1패턴을 사용해hasNext판단을 올바르게 지원합니다.
110-136: LGTM!응답 변환 로직이 효율적이고 정확하게 구현되어 있습니다:
- 페이지네이션 처리가 올바름
- 멤버 캐시 데이터를 배치로 조회하여 N+1 문제 방지
- 탈퇴한 멤버에 대한 폴백 처리 포함
...n/java/com/cozymate/cozymate_server/domain/fcm/event/listener/NotificationEventListener.java
Show resolved
Hide resolved
| @Slf4j | ||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class ChatRoomService { |
There was a problem hiding this comment.
ChatRoom, ChatRooMember 관련 클래스를 각각 만들기엔 ChatRoomMember 쪽 양이 적기도하고, ChatRoomMember 관련해서는 방 알림 끄기가 전부인데 ChatRoomService에 있어도 될 것 같아서, ChatRoomXXX(Service, RepositoryService, Converter) 클래스에 합쳐서 구현했습니다
|
Mongo Atlas정보 Notion에 부탁드려요 |
src/main/java/com/cozymate/cozymate_server/domain/member/controller/MemberController.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class MemberCacheService { |
There was a problem hiding this comment.
사용자 정보를 캐싱하는 건 채팅방 때문인거 같은데, dto하나가 만료된 상황에 여러명이 동시에 조회하면 다들 miss나는 건가요? 추후에 개선하면 좋을거 같아요. 한 번 미스나서 배치하는 동안에는 락거는 방향으로
src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisPublisher.java
Show resolved
Hide resolved
| try { | ||
| chatPubDTO = objectMapper.readValue(message.getBody(), ChatPubDTO.class); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
로그 찍고 재시도 큐 같은데다 박는 구조가 어떨까요.
| throw new RuntimeException(e); | ||
| } | ||
|
|
||
| simpMessagingTemplate.convertAndSend( |
There was a problem hiding this comment.
여기서 바로 호출하면 레디스 뻑나면 스레드 고갈 날 수도 있는 건가요? 전용 쓰레드풀로 Execute 시키는게 어떤지요
There was a problem hiding this comment.
simpMessagingTemplate.convertAndSend() 작업을 하는 스레드 풀을 따로 관리하자는 말씀이신걸가요??
src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisSubscriberManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cozymate/cozymate_server/global/redispubsub/config/RedisPubSubConfig.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/com/cozymate/cozymate_server/domain/member/service/MemberService.java (2)
130-136: 캐시 업데이트 패턴이 올바르게 구현되었습니다.회원 정보 수정 후 캐시를 업데이트하는 로직이 적절하게 구현되었습니다:
- Try-catch로 캐시 실패 시에도 메인 로직이 중단되지 않도록 방어적으로 처리
- 실패 시 로그를 남겨 추적 가능성 확보
- TTL(1시간)이 있어 일시적인 불일치가 자동으로 해소됨
선택적 개선사항: 트랜잭션 커밋 후 캐시를 업데이트하면(예:
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)사용) 트랜잭션 실패 시 stale cache를 완전히 방지할 수 있습니다. 하지만 현재 구현도 캐시 작업이 메서드 마지막에 위치하고 TTL이 있어 충분히 안전합니다.
152-158: 캐시 삭제 로직이 올바르게 구현되었습니다.회원 탈퇴 후 캐시를 삭제하는 로직이 update 메서드와 일관된 패턴으로 적절하게 구현되었습니다:
- Try-catch로 캐시 실패 시에도 탈퇴 프로세스가 중단되지 않도록 처리
- 탈퇴 시 캐시 삭제는 올바른 접근 방식 (탈퇴 마커 저장 대신)
Update 메서드와 동일하게, 트랜잭션 커밋 후 캐시 작업을 수행하는 것을 선택적으로 고려할 수 있지만, 현재 구현도 충분히 안전합니다.
src/main/java/com/cozymate/cozymate_server/global/redispubsub/config/RedisPubSubConfig.java (1)
34-48: 스레드 풀 설정 재검토 및 추가 옵션 고려를 권장합니다.현재 설정(core=2, max=3, queue=200)은 소규모 트래픽에는 적합하지만, 채팅 시스템의 예상 부하에 따라 부족할 수 있습니다. 또한
CallerRunsPolicy는 큐가 가득 찰 경우 Redis 구독 스레드가 직접 메시지를 처리하게 되어 구독 처리가 블로킹될 위험이 있습니다.다음 사항을 검토해 주세요:
- 스레드 풀 크기: 동시 활성 채팅방 수와 메시지 처리량을 고려하여 조정
- Rejection Policy:
AbortPolicy+ DLQ 또는 모니터링 강화 검토- 추가 옵션: 이전 리뷰 코멘트에서 언급된 대로,
RedisMessageListenerContainer에 구독 관리용 별도 스레드 풀(setSubscriptionExecutor) 설정을 고려예시:
@Bean public Executor redisListenerExecutor() { ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); executor.setCorePoolSize(5); // 증가 고려 executor.setMaxPoolSize(10); // 증가 고려 executor.setQueueCapacity(500); // 증가 고려 // rejection policy는 요구사항에 따라 선택 executor.setRejectedExecutionHandler(new CallerRunsPolicy()); // ... 나머지 설정 return executor; } // 선택사항: 구독 관리용 별도 executor @Bean public Executor redisSubscriptionExecutor() { ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); executor.setCorePoolSize(2); executor.setThreadNamePrefix("redis-subscription-"); executor.initialize(); return executor; } @Bean public RedisMessageListenerContainer redisMessageListenerContainer() { // ... container.setSubscriptionExecutor(redisSubscriptionExecutor()); // ... }Based on past review comments
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/cozymate/cozymate_server/domain/member/controller/MemberController.java(0 hunks)src/main/java/com/cozymate/cozymate_server/domain/member/service/MemberService.java(3 hunks)src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisSubscriberManager.java(1 hunks)src/main/java/com/cozymate/cozymate_server/global/redispubsub/config/RedisPubSubConfig.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/cozymate/cozymate_server/domain/member/controller/MemberController.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/cozymate/cozymate_server/global/redispubsub/RedisSubscriberManager.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/cozymate/cozymate_server/domain/member/service/MemberService.java (1)
src/main/java/com/cozymate/cozymate_server/domain/member/converter/MemberConverter.java (1)
MemberConverter(16-117)
🔇 Additional comments (1)
src/main/java/com/cozymate/cozymate_server/domain/member/service/MemberService.java (1)
42-42: LGTM! 의존성 주입이 올바르게 추가되었습니다.
MemberCacheService의존성이 표준 Spring DI 패턴에 따라 올바르게 추가되었습니다.
| @Bean | ||
| public RedisMessageListenerContainer redisMessageListenerContainer() { | ||
| RedisMessageListenerContainer container = new RedisMessageListenerContainer(); | ||
| container.setConnectionFactory(redisConnectionFactory); | ||
| container.setTaskExecutor(redisListenerExecutor()); | ||
| return container; | ||
| } |
There was a problem hiding this comment.
에러 핸들러 추가를 권장합니다.
RedisMessageListenerContainer에 명시적인 에러 핸들러가 설정되어 있지 않습니다. 메시지 처리 중 예외 발생 시 로그만 남기고 메시지가 유실될 수 있으며, 에러 패턴 파악이나 알림 연동이 어렵습니다.
다음 설정을 추가하여 에러 처리 전략을 명시하는 것을 권장합니다:
@Bean
public RedisMessageListenerContainer redisMessageListenerContainer() {
RedisMessageListenerContainer container = new RedisMessageListenerContainer();
container.setConnectionFactory(redisConnectionFactory);
container.setTaskExecutor(redisListenerExecutor());
+ container.setErrorHandler(t -> {
+ log.error("Redis Pub/Sub message processing failed", t);
+ // 필요시 알림 전송, 메트릭 수집 등 추가
+ });
return container;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Bean | |
| public RedisMessageListenerContainer redisMessageListenerContainer() { | |
| RedisMessageListenerContainer container = new RedisMessageListenerContainer(); | |
| container.setConnectionFactory(redisConnectionFactory); | |
| container.setTaskExecutor(redisListenerExecutor()); | |
| return container; | |
| } | |
| @Bean | |
| public RedisMessageListenerContainer redisMessageListenerContainer() { | |
| RedisMessageListenerContainer container = new RedisMessageListenerContainer(); | |
| container.setConnectionFactory(redisConnectionFactory); | |
| container.setTaskExecutor(redisListenerExecutor()); | |
| container.setErrorHandler(t -> { | |
| log.error("Redis Pub/Sub message processing failed", t); | |
| // 필요시 알림 전송, 메트릭 수집 등 추가 | |
| }); | |
| return container; | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/cozymate/cozymate_server/global/redispubsub/config/RedisPubSubConfig.java
around lines 26 to 32, the RedisMessageListenerContainer is created without an
explicit error handler; update the bean to set a custom ErrorHandler (or provide
a lambda implementing ErrorHandler) that logs exceptions with contextual details
and forwards critical failures to your monitoring/alerting system, and
optionally implements simple retry/backoff or delegates to a retrying task
executor; ensure the error handler is injected or created as a bean and wired
into container.setErrorHandler(...) so exceptions during message processing are
captured and handled instead of being silently lost.
genius00hwan
left a comment
genius00hwan
left a comment
There was a problem hiding this comment.
수고하셨습니다. 근데 어떤 문제가 있을지 몰라서 채팅만큼은 개발서버에 올리고 테스트 진행후에 배포해도 될것같다는 생각이..
eple0329
left a comment
eple0329
left a comment
There was a problem hiding this comment.
너무 길어요 ㅠ
보다가 끝이 없는 코드 끝에 지쳐... 일단 쭉 보다 서비스, 엔티티, 레포 위주로 확인했습니당.
고생하셨어요... 문제생겨도 다른 도메인에 영향을 크게 주진 않을거같네요
| @Document("chat") | ||
| public class Chat { | ||
|
|
||
| @Id | ||
| private String id; // MongoDB ObjectId | ||
| private Long chatRoomId; // 채팅방 ID | ||
| private Long memberId; // 메시지 보낸 사람 | ||
| private String content; // 메시지 내용 | ||
| private LocalDateTime createdAt; // 생성 시간 | ||
| } |
There was a problem hiding this comment.
roomId랑 createdAt으로 검색하니까, 인덱스 걸어두는게 어떨까용
⚒️develop의 최신 커밋을 pull 받았나요?
#️⃣ 작업 내용
채팅 구현
Redis 사용 전략
수정된 람다 코드(send_each() 추가) (배포되면 변경 예정)
동작 확인
채팅방에 5명 존재
채팅방에 현재 2명 접속
한명이 채팅을 보내면 나머지 3명에게 알림 전송
미접속 사용자 중 한명이 알림 수신 off
2명에게 알림 전송
수정 전
수정 후
7-2 member2가 전송한 채팅이 @Valid 제약에 걸림
8-2 stomp subscribe 시 사용자 조회가 안되는 경우
8-3 다른 대학 기숙사 채팅방을 stomp subscribe 하려는 경우
💬 리뷰 요구사항(선택)
Summary by CodeRabbit
신기능
개선
작업