[FEAT] 채팅 관련 REST API 구현 #61
Conversation
|
Warning Rate limit exceeded@buzz0331 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughMongoDB 기반 채팅 저장소와 WebSocket(STOMP) 통신·인증·리졸버를 추가하고, 채팅 도메인/레포지토리/서비스/컨트롤러와 DTO를 도입하며 Reservation 도메인에 채팅방 연동 및 빌드·설정(MongoDB, WebSocket) 의존성을 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatRoomService
participant FoodTruckRepo as FoodTruckRepository
participant ChatRoomRepo as ChatRoomRepository
participant ChatRoomMetaRepo as ChatRoomMetaDataRepository
Client->>ChatRoomService: createChatRoom(member, foodTruckId)
ChatRoomService->>FoodTruckRepo: findById(foodTruckId)
FoodTruckRepo-->>ChatRoomService: FoodTruck
ChatRoomService->>ChatRoomRepo: findByMemberAndFoodTruck(member, foodTruck)
alt exists
ChatRoomRepo-->>ChatRoomService: existing ChatRoom
else
ChatRoomService->>ChatRoomRepo: save(new ChatRoom)
ChatRoomRepo-->>ChatRoomService: ChatRoom
end
ChatRoomService->>ChatRoomMetaRepo: findByRoomId(roomId)
alt meta missing
ChatRoomService->>ChatRoomMetaRepo: save(ChatRoomMetaData.from(...))
end
ChatRoomService-->>Client: ChatRoomIdResponse
sequenceDiagram
participant WSClient
participant StompInterceptor as StompAuthChannelInterceptor
participant JwtUtil
participant StompResolver as StompUserIdArgumentResolver
participant ChatController
participant ChatService
participant Broker
WSClient->>StompInterceptor: STOMP CONNECT (Authorization)
StompInterceptor->>JwtUtil: extractLoginUser(token)
JwtUtil-->>StompInterceptor: LoginUser(userId)
StompInterceptor->>StompInterceptor: session.setAttribute(JWT_ACCESS_TOKEN_KEY,userId)
WSClient->>ChatController: SEND /pub/rooms/{roomId} (message)
ChatController->>StompResolver: resolve `@UserId`
StompResolver-->>ChatController: userId
ChatController->>ChatService: sendMessage(roomId, userId, request)
ChatService-->>ChatController: ChatMessageResponse
ChatController->>Broker: messagingTemplate.convertAndSend(/sub/rooms/{roomId}, response)
Broker-->>WSClient: broadcast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
build.gradle (1)
78-83: 주석 처리된 의존성 제거 고려MongoDB와 WebSocket 의존성 추가는 적절합니다. 다만 Line 83의 주석 처리된
spring-boot-starter-messaging의존성은 필요하지 않다면 제거하는 것이 코드베이스를 깔끔하게 유지하는 데 도움이 됩니다.src/main/java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepository.java (1)
3-5: 읽음 처리 방식에 대한 피드백인터페이스 정의는 명확합니다. PR 설명에서 언급하신 성능 우려(입장 시 전체 메시지 일괄 읽음 처리)와 관련하여 몇 가지 대안을 제안드립니다:
- 현재 방식 (동기 일괄 처리): 구현이 단순하지만 메시지 수가 많을 경우 응답 지연 가능
- 비동기 이벤트 기반: Spring ApplicationEvent를 발행하고
@Async로 처리 - 사용자 경험 개선- 최근 접속 시간 기반: 사용자별 마지막 접속 시간을 저장하고 해당 시점 이후 메시지만 읽음 처리 - DB 부하 감소
메시지 수가 제한적이라면 현재 방식도 충분하지만, 향후 확장성을 고려하면 비동기 이벤트 방식을 추천드립니다.
src/main/java/konkuk/chacall/global/common/security/resolver/StompUserIdArgumentResolver.java (1)
21-44:long타입도 인식하도록 보완 권장컨트롤러에서
@UserId long userId처럼 primitive 타입을 사용하면 현재 조건(Long.class만 허용) 때문에 ArgumentResolver가 동작하지 않습니다. Wrapper와 primitive를 모두 허용하도록 조건을 확장해 주세요.다음처럼 수정할 수 있습니다.
- return parameter.hasParameterAnnotation(UserId.class) - && parameter.getParameterType().equals(Long.class); + if (!parameter.hasParameterAnnotation(UserId.class)) { + return false; + } + Class<?> type = parameter.getParameterType(); + return Long.class.equals(type) || long.class.equals(type);src/main/java/konkuk/chacall/domain/chat/application/message/ChatMessageService.java (1)
48-60: 오프셋 기반 페이징 대신 커서 기반 페이징 도입을 고려하세요.현재
PageRequest와Sort.Direction.DESC를 사용한 오프셋 기반 페이징은 MongoDB에서 성능상 비효율적일 수 있습니다. PR 설명에서 언급하신 것처럼 향후 커서 기반 페이징으로 리팩토링 계획이 있다면, 코드에 TODO 주석을 추가하여 개선 사항을 명시하는 것을 권장합니다.다음 diff를 적용하여 TODO 주석을 추가하세요:
public List<ChatMessageResponse> getChatMessages(Long roomId, User user, int page, int size) { ChatRoomMetaData chatRoomMetaData = chatRoomMetaDataRepository.findByRoomId(roomId) .orElseThrow(() -> new EntityNotFoundException(CHAT_ROOM_NOT_FOUND)); chatRoomMetaData.validateParticipant(user); + // TODO: MongoDB 특성을 고려하여 커서 기반 페이징으로 리팩토링 필요 var pageable = PageRequest.of(page, size, Sort.by(Sort.Direction.DESC, "sendTime"));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
build.gradle(1 hunks)src/main/java/konkuk/chacall/domain/chat/application/ChatService.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/application/message/ChatMessageService.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/domain/ChatMessage.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java(2 hunks)src/main/java/konkuk/chacall/domain/chat/domain/ChatRoomMetaData.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/domain/repository/ChatMessageRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/domain/repository/ChatRoomMetaDataRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/domain/repository/ChatRoomRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/domain/repository/dto/ChatRoomMetaDataProjection.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepositoryImpl.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/ChatController.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/ChatRestController.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/dto/request/CreateChatRoomRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/dto/request/GetChatRoomRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/dto/request/SendChatMessageRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatMessageResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatOpponentResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomIdResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomResponse.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/security/interceptor/StompAuthChannelInterceptor.java(1 hunks)src/main/java/konkuk/chacall/global/common/security/resolver/StompUserIdArgumentResolver.java(1 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(1 hunks)src/main/java/konkuk/chacall/global/common/util/DateUtil.java(1 hunks)src/main/java/konkuk/chacall/global/config/WebSocketConfig.java(1 hunks)src/main/resources/application-dev.yml(1 hunks)src/main/resources/application-local.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomResponse.java (1)
src/main/java/konkuk/chacall/global/common/util/DateUtil.java (1)
DateUtil(6-32)
src/main/java/konkuk/chacall/domain/chat/application/ChatService.java (3)
src/main/java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepositoryImpl.java (1)
RequiredArgsConstructor(10-29)src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java (1)
Service(28-109)src/main/java/konkuk/chacall/domain/chat/application/message/ChatMessageService.java (1)
Service(20-70)
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java (2)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/chat/application/ChatService.java (1)
Service(22-71)
src/main/java/konkuk/chacall/global/common/security/interceptor/StompAuthChannelInterceptor.java (1)
src/main/java/konkuk/chacall/global/common/security/resolver/StompUserIdArgumentResolver.java (1)
Component(17-46)
src/main/java/konkuk/chacall/domain/chat/presentation/ChatRestController.java (1)
src/main/java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepositoryImpl.java (1)
RequiredArgsConstructor(10-29)
src/main/java/konkuk/chacall/domain/chat/application/message/ChatMessageService.java (3)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepositoryImpl.java (1)
RequiredArgsConstructor(10-29)src/main/java/konkuk/chacall/domain/chat/application/ChatService.java (1)
Service(22-71)
src/main/java/konkuk/chacall/domain/chat/domain/ChatMessage.java (1)
src/main/java/konkuk/chacall/domain/user/domain/model/User.java (1)
Getter(9-86)
src/main/java/konkuk/chacall/global/common/security/resolver/StompUserIdArgumentResolver.java (1)
src/main/java/konkuk/chacall/global/common/security/interceptor/StompAuthChannelInterceptor.java (1)
Component(18-46)
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java (3)
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoomMetaData.java (1)
Getter(15-79)src/main/java/konkuk/chacall/domain/user/domain/model/User.java (1)
Getter(9-86)src/main/java/konkuk/chacall/domain/member/domain/SavedFoodTruck.java (1)
Builder(9-36)
🔇 Additional comments (15)
src/main/resources/application-dev.yml (1)
24-26: MongoDB 설정 검증:uri와database속성의 버전별 동작 확인 필요.spring.data.mongodb.database 속성이 URI에 포함된 데이터베이스보다 우선적으로 적용되는 경우가 있으며, Spring Boot 3에서는 동작이 달라져 spring.data.mongodb.database가 무시될 수 있습니다. 현재 dev 프로필에서
uri와database를 함께 사용하고 있으므로 다음을 확인하세요:
- 사용 중인 Spring Boot 버전 확인
- MongoDB URI에 이미 데이터베이스명 포함 여부 확인 (
mongodb://host:port/dbname형식)- 포함되어 있다면 별도의
database속성 제거 고려 (모든 연결 정보를 URI에 통합하는 것이 표준)- 프로필 간 설정 일관성 검토 (dev는 URI 방식, local은 host/port 방식)
src/main/java/konkuk/chacall/domain/chat/presentation/dto/request/GetChatRoomRequest.java (1)
9-17: LGTM!
isOwner필드를 필수로 받는 설계가 PR 설명의 요구사항(프론트엔드에서 뷰 정보 전달)과 일치하며, 페이징 인프라와의 통합도 적절합니다.src/main/java/konkuk/chacall/domain/chat/presentation/dto/request/CreateChatRoomRequest.java (1)
6-11: LGTM!간결하고 명확한 DTO 설계입니다. 필수 검증과 문서화가 적절히 적용되어 있습니다.
src/main/resources/application-local.yml (1)
24-30: MongoDB 설정 적절함환경 변수를 통한 인증 정보 관리가 올바르게 구성되었습니다. 로컬 개발 환경에 적합한 설정입니다.
src/main/java/konkuk/chacall/domain/chat/application/ChatService.java (2)
22-24: 트랜잭션 설정 적절함클래스 레벨의
@Transactional(readOnly = true)와 쓰기 메서드별@Transactional오버라이드는 모범 사례입니다.
65-70: 읽음 처리 트랜잭션 범위 확인
markMessagesAsRead메서드에@Transactional이 적절히 적용되어 있습니다. MongoDB 쓰기 작업에 대한 트랜잭션 처리가 보장됩니다.다만 MongoDB의 단일 컬렉션 업데이트는 원자적이므로, 여러 컬렉션을 동시에 업데이트하지 않는다면 트랜잭션이 필수는 아닙니다. 현재 구조에서는 안전성을 위해 유지하는 것이 합리적입니다.
src/main/java/konkuk/chacall/domain/chat/domain/repository/dto/ChatRoomMetaDataProjection.java (1)
5-17: LGTM!MongoDB 집계 결과를 위한 프로젝션 인터페이스 설계가 적절합니다.
unreadCount를 집계로 계산하여 RDB의 반정규화를 피하는 접근 방식이 PR 목표와 일치합니다.src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatOpponentResponse.java (1)
5-14: DTO 구조 명확합니다레코드 활용으로 응답 스펙이 간결하고 가독성이 좋습니다. 그대로 진행해도 될 듯합니다.
src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatMessageResponse.java (1)
7-24: 엔터티-DTO 매핑 잘 구성되었습니다도메인 객체에서 필요한 필드만 그대로 옮겨와 응답 구조가 명확합니다.
src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomIdResponse.java (1)
6-13: 간결하고 명확한 DTO 구현입니다.record 패턴과 정적 팩토리 메서드를 활용한 깔끔한 구현입니다.
src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java (1)
268-279: Swagger 응답 정의가 일관성 있게 추가되었습니다.기존 패턴을 잘 따르고 있으며, 새로운 채팅 관련 에러 코드와 올바르게 연동되어 있습니다.
src/main/java/konkuk/chacall/domain/chat/domain/repository/ChatRoomRepository.java (1)
17-18: N+1 문제를 효과적으로 방지하고 있습니다.
@EntityGraph를 활용하여 member와 foodTruck을 함께 로딩하는 것은 배치 조회 시 성능을 최적화하는 좋은 접근입니다.src/main/java/konkuk/chacall/domain/chat/presentation/ChatController.java (1)
23-31: WebSocket 메시지 처리 로직이 명확합니다.STOMP 메시지 수신 후 서비스 레이어를 통해 처리하고 구독자들에게 브로드캐스트하는 흐름이 잘 구현되어 있습니다.
src/main/java/konkuk/chacall/domain/chat/application/message/ChatMessageService.java (1)
27-46: 메시지 전송 로직이 잘 구조화되어 있습니다.참여자 검증, 메시지 생성 및 저장, 메타데이터 업데이트가 명확한 흐름으로 구현되어 있습니다.
src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
116-124: 채팅방 관련 에러 코드가 일관되게 추가되었습니다.새로운 ChatRoom 섹션의 에러 코드들이 기존 패턴을 잘 따르고 있으며, HTTP 상태 코드와 메시지가 적절하게 정의되어 있습니다.
| public ChatOpponentResponse getChatOpponentName(User user, Long roomId, boolean isOwner) { | ||
| ChatRoom chatRoom = chatRoomRepository.findById(roomId) | ||
| .orElseThrow(() -> new EntityNotFoundException(CHAT_ROOM_NOT_FOUND)); | ||
|
|
||
| if(isOwner && user.getRole() != Role.OWNER) { | ||
| throw new BusinessException(USER_FORBIDDEN); | ||
| } | ||
|
|
||
| // 푸드트럭 사장일 경우 예약자 이름 반환 | ||
| if(isOwner) return ChatOpponentResponse.of(chatRoom.getMember().getName(), null); | ||
|
|
||
| // 예약자일 경우 푸드트럭 사장 이름 및 푸드트럭 이름 반환 | ||
| FoodTruck foodTruck = chatRoom.getFoodTruck(); | ||
| return ChatOpponentResponse.of(foodTruck.getOwner().getName(), foodTruck.getFoodTruckInfo().getName()); | ||
| } |
There was a problem hiding this comment.
참여자 검증 누락으로 정보 노출 가능
getChatOpponentName은 user가 해당 채팅방의 예약자/사장인지 확인하지 않습니다. 인증된 다른 사용자가 임의의 roomId로 호출하면 상대 이름과 푸드트럭 이름을 열람할 수 있어 정보 노출 취약점이 됩니다. chatRoom의 참여자 ID와 user.getUserId()를 비교해 불일치 시 CHAT_ROOM_FORBIDDEN 등을 던지도록 검증을 추가해 주세요.
+ Long ownerId = chatRoom.getFoodTruck().getOwner().getUserId();
+ Long memberId = chatRoom.getMember().getUserId();
+ if (!ownerId.equals(user.getUserId()) && !memberId.equals(user.getUserId())) {
+ throw new BusinessException(CHAT_ROOM_FORBIDDEN);
+ }🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java
around lines 55 to 69, add participant validation: after loading chatRoom,
verify that the authenticated user is actually a participant of that room and
throw a BusinessException with CHAT_ROOM_FORBIDDEN if not. Concretely, if
isOwner is true ensure user.getRole()==Role.OWNER and user.getUserId() equals
chatRoom.getFoodTruck().getOwner().getId(); if isOwner is false ensure
user.getUserId() equals chatRoom.getMember().getId(); on mismatch throw new
BusinessException(CHAT_ROOM_FORBIDDEN). This prevents arbitrary users from
reading opponent/foodtruck info.
| public CursorPagingResponse<ChatRoomResponse> getChatRooms(User member, Boolean isOwner, Long cursor, Integer size) { | ||
| int pageSize = (size == null || size < 1) ? 20 : size; | ||
|
|
||
| Long cursorSortKey = (cursor != null) ? cursor : Long.MAX_VALUE; | ||
| int limit = pageSize + 1; | ||
|
|
||
| // 1. MongoDB에서 메타데이터 + unreadCount 조회 | ||
| List<ChatRoomMetaDataProjection> metaList = | ||
| chatRoomMetaDataRepository.findChatRoomsForUser(member.getUserId(), isOwner, cursorSortKey, limit); | ||
|
|
||
| boolean hasNext = metaList.size() > pageSize; | ||
| if (hasNext) { | ||
| metaList = metaList.subList(0, pageSize); | ||
| } | ||
|
|
||
| // 2. roomId 리스트로 RDB에서 ChatRoom 배치 조회 | ||
| List<Long> roomIds = metaList.stream() | ||
| .map(ChatRoomMetaDataProjection::getRoomId) | ||
| .toList(); | ||
|
|
||
| List<ChatRoom> chatRooms = chatRoomRepository.findByChatRoomIdIn(roomIds); | ||
| Map<Long, ChatRoom> chatRoomMap = chatRooms.stream() | ||
| .collect(Collectors.toMap(ChatRoom::getChatRoomId, Function.identity())); | ||
|
|
||
| // 3. 메타데이터 + RDB 정보 조합해서 ChatRoomResponse 생성 | ||
| List<ChatRoomResponse> responses = metaList.stream() | ||
| .map(meta -> ChatRoomResponse.from(chatRoomMap.get(meta.getRoomId()), meta, isOwner)) | ||
| .toList(); | ||
|
|
||
| // 4. CursorPagingResponse 생성 | ||
| Long lastCursor = responses.isEmpty() ? null : metaList.get(metaList.size() - 1).getSortKey(); | ||
| return new CursorPagingResponse<>( | ||
| responses, | ||
| lastCursor, | ||
| hasNext, | ||
| null | ||
| ); | ||
| } |
There was a problem hiding this comment.
isOwner null 처리 필요
getChatRooms의 isOwner는 래퍼 타입인데 그대로 findChatRoomsForUser에 넘기면 자동 언박싱이 일어나므로, 요청 파라미터가 비어 있을 경우 즉시 NullPointerException이 발생합니다. 기본값을 안전하게 계산한 뒤 그 값을 전체 로직에서 사용하도록 변경해 주세요.
- List<ChatRoomMetaDataProjection> metaList =
- chatRoomMetaDataRepository.findChatRoomsForUser(member.getUserId(), isOwner, cursorSortKey, limit);
+ boolean ownerView = Boolean.TRUE.equals(isOwner);
+
+ List<ChatRoomMetaDataProjection> metaList =
+ chatRoomMetaDataRepository.findChatRoomsForUser(member.getUserId(), ownerView, cursorSortKey, limit);
...
- .map(meta -> ChatRoomResponse.from(chatRoomMap.get(meta.getRoomId()), meta, isOwner))
+ .map(meta -> ChatRoomResponse.from(chatRoomMap.get(meta.getRoomId()), meta, ownerView))| // 1. 내가 속한 채팅방만 필터링 (isOwner 기준) | ||
| "{ '$match': { '$expr': { '$and': [" + | ||
| " { '$cond': [ ?1, { '$eq': ['$ownerId', ?0] }, { '$eq': ['$memberId', ?0] } ] }," + | ||
| " { '$lt': ['$sortKey', ?2] }" + | ||
| "] } } }", | ||
|
|
||
| // 2. 안읽은 메시지 개수 계산 (chat_messages 컬렉션 join) | ||
| "{ '$lookup': { " + | ||
| " 'from': 'chat_messages'," + | ||
| " 'let': { 'roomId': '$roomId' }," + | ||
| " 'pipeline': [" + | ||
| " { '$match': { '$expr': { '$and': [" + | ||
| " { '$eq': ['$roomId', '$$roomId'] }," + | ||
| " { '$eq': ['$read', false] }," + | ||
| " { '$ne': ['$senderId', ?0] }" + | ||
| " ] } } }," + | ||
| " { '$count': 'unreadCount' }" + | ||
| " ]," + | ||
| " 'as': 'unreadInfo'" + | ||
| "} }", | ||
|
|
||
| // 3. unreadInfo 배열 -> unreadCount 필드로 변환 (없으면 0) | ||
| "{ '$addFields': { " + | ||
| " 'unreadCount': { '$ifNull': [ { '$arrayElemAt': ['$unreadInfo.unreadCount', 0] }, 0 ] }" + | ||
| "} }", | ||
|
|
||
| // 4. sortKey 기준 내림차순 정렬 (가장 최근 대화가 위로) | ||
| "{ '$sort': { 'sortKey': -1 } }", | ||
|
|
||
| // 5. 페이지 사이즈 + 1 만큼만 가져와서 hasNext 판단 | ||
| "{ '$limit': ?3 }" | ||
| }) |
There was a problem hiding this comment.
첫 페이지에서 sortKey 필터 조건 보완 필요
createChatRoom에서 새 메타데이터의 sortKey를 Long.MAX_VALUE로 초기화하는데, 첫 페이지 조회 시 전달되는 cursorSortKey도 동일 값입니다. 현재 $match 단계가 $lt 조건을 사용해 동일한 sortKey를 모두 탈락시키므로, 첫 메시지 전인 신규 채팅방이 목록에서 완전히 누락됩니다. $lte로 변경해 동일한 sortKey도 포함시켜 주세요.
- " { '$lt': ['$sortKey', ?2] }" +
+ " { '$lte': ['$sortKey', ?2] }" +📝 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.
| // 1. 내가 속한 채팅방만 필터링 (isOwner 기준) | |
| "{ '$match': { '$expr': { '$and': [" + | |
| " { '$cond': [ ?1, { '$eq': ['$ownerId', ?0] }, { '$eq': ['$memberId', ?0] } ] }," + | |
| " { '$lt': ['$sortKey', ?2] }" + | |
| "] } } }", | |
| // 2. 안읽은 메시지 개수 계산 (chat_messages 컬렉션 join) | |
| "{ '$lookup': { " + | |
| " 'from': 'chat_messages'," + | |
| " 'let': { 'roomId': '$roomId' }," + | |
| " 'pipeline': [" + | |
| " { '$match': { '$expr': { '$and': [" + | |
| " { '$eq': ['$roomId', '$$roomId'] }," + | |
| " { '$eq': ['$read', false] }," + | |
| " { '$ne': ['$senderId', ?0] }" + | |
| " ] } } }," + | |
| " { '$count': 'unreadCount' }" + | |
| " ]," + | |
| " 'as': 'unreadInfo'" + | |
| "} }", | |
| // 3. unreadInfo 배열 -> unreadCount 필드로 변환 (없으면 0) | |
| "{ '$addFields': { " + | |
| " 'unreadCount': { '$ifNull': [ { '$arrayElemAt': ['$unreadInfo.unreadCount', 0] }, 0 ] }" + | |
| "} }", | |
| // 4. sortKey 기준 내림차순 정렬 (가장 최근 대화가 위로) | |
| "{ '$sort': { 'sortKey': -1 } }", | |
| // 5. 페이지 사이즈 + 1 만큼만 가져와서 hasNext 판단 | |
| "{ '$limit': ?3 }" | |
| }) | |
| // 1. 내가 속한 채팅방만 필터링 (isOwner 기준) | |
| "{ '$match': { '$expr': { '$and': [" + | |
| " { '$cond': [ ?1, { '$eq': ['$ownerId', ?0] }, { '$eq': ['$memberId', ?0] } ] }," + | |
| " { '$lte': ['$sortKey', ?2] }" + | |
| "] } } }", | |
| // 2. 안읽은 메시지 개수 계산 (chat_messages 컬렉션 join) | |
| "{ '$lookup': { " + | |
| " 'from': 'chat_messages'," + | |
| " 'let': { 'roomId': '$roomId' }," + | |
| " 'pipeline': [" + | |
| " { '$match': { '$expr': { '$and': [" + | |
| " { '$eq': ['$roomId', '$$roomId'] }," + | |
| " { '$eq': ['$read', false] }," + | |
| " { '$ne': ['$senderId', ?0] }" + | |
| " ] } } }," + | |
| " { '$count': 'unreadCount' }" + | |
| " ]," + | |
| " 'as': 'unreadInfo'" + | |
| "} }", | |
| // 3. unreadInfo 배열 -> unreadCount 필드로 변환 (없으면 0) | |
| "{ '$addFields': { " + | |
| " 'unreadCount': { '$ifNull': [ { '$arrayElemAt': ['$unreadInfo.unreadCount', 0] }, 0 ] }" + | |
| "} }", | |
| // 4. sortKey 기준 내림차순 정렬 (가장 최근 대화가 위로) | |
| "{ '$sort': { 'sortKey': -1 } }", | |
| // 5. 페이지 사이즈 + 1 만큼만 가져와서 hasNext 판단 | |
| "{ '$limit': ?3 }" | |
| }) |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/chat/domain/repository/ChatRoomMetaDataRepository.java
around lines 21 to 52, the aggregation $match uses '$lt' on sortKey which
excludes new chat rooms initialized with Long.MAX_VALUE when cursorSortKey is
also Long.MAX_VALUE; change the comparison operator to '$lte' so entries with
equal sortKey are included for the first page, ensuring newly created rooms
appear in results.
There was a problem hiding this comment.
근데 이렇게 하면, 다음 페이지 조회시에 중복된 채팅방이 조회되지 않나?
There was a problem hiding this comment.
@buzz0331, 정확한 지적입니다! 👍
$lte로 변경하면 동일한 sortKey를 가진 채팅방들이 여러 페이지에 걸쳐 중복 조회될 수 있습니다. 특히 메시지가 없는 신규 채팅방들이 모두 Long.MAX_VALUE를 가지는 경우 이 문제가 발생합니다.
문제 시나리오:
- 페이지 1: sortKey = Long.MAX_VALUE인 방 3개 중 2개 반환
- lastCursor = Long.MAX_VALUE
- 페이지 2: sortKey <= Long.MAX_VALUE 조건으로 다시 조회 → 이전에 본 방들 재등장
올바른 해결 방법:
MongoDB cursor pagination에서 sortKey가 unique하지 않으면 결과가 부정확해지므로, roomId를 보조 필드로 사용하는 복합 커서를 구현해야 합니다:
// 1. 커서를 (sortKey, roomId) 복합으로 변경
// ChatRoomService.getChatRooms에서:
Long cursorSortKey = cursor != null ? cursor : Long.MAX_VALUE;
Long cursorRoomId = cursor != null ? cursorRoomId : Long.MAX_VALUE;
// 2. Aggregation 쿼리 수정:
@Aggregation(pipeline = {
"{ '$match': { '$expr': { '$and': [" +
" { '$cond': [ ?1, { '$eq': ['$ownerId', ?0] }, { '$eq': ['$memberId', ?0] } ] }," +
" { '$or': [" +
" { '$lt': ['$sortKey', ?2] }," + // sortKey < cursor
" { '$and': [" +
" { '$eq': ['$sortKey', ?2] }," + // sortKey == cursor AND
" { '$lt': ['$roomId', ?3] }" + // roomId < cursorRoomId
" ] }" +
" ] }" +
"] } } }",
// ... 나머지 파이프라인
"{ '$sort': { 'sortKey': -1, 'roomId': -1 } }", // 복합 정렬
"{ '$limit': ?4 }"
})
List<ChatRoomMetaDataProjection> findChatRoomsForUser(
Long userId, boolean isOwner, Long cursorSortKey, Long cursorRoomId, int limit);이렇게 하면:
- 동일한
sortKey를 가진 방들 사이에서도roomId로 명확한 순서가 보장됩니다 - 중복 없이 정확한 페이지네이션이 가능합니다
- 첫 페이지(
Long.MAX_VALUE)도 정상 작동합니다
또는 더 간단한 대안:
현재 상황에서 sortKey 충돌이 주로 빈 채팅방(Long.MAX_VALUE)에서만 발생한다면, 채팅방 생성 시 sortKey를 Long.MAX_VALUE - roomId로 초기화하여 unique하게 만드는 방법도 고려할 수 있습니다.
어떤 접근 방식이 더 적합할까요?
There was a problem hiding this comment.
오호 그러면 채팅방 생성시에 Long.MAX_VALUE - roomId로 초기화하도록 할게.
There was a problem hiding this comment.
@buzz0331, 좋은 선택입니다! 👍
sortKey를 Long.MAX_VALUE - roomId로 초기화하면:
- 각 채팅방마다 고유한
sortKey가 보장됩니다 - 기존
$lt조건으로도 중복 없이 정확한 페이지네이션이 가능합니다 - 복합 커서 구현 없이도 간단하게 문제를 해결할 수 있습니다
변경이 필요한 위치는 ChatRoomMetaData.from() 메서드입니다:
public static ChatRoomMetaData from(ChatRoom chatRoom) {
return ChatRoomMetaData.builder()
.roomId(chatRoom.getChatRoomId())
.memberId(chatRoom.getMember().getUserId())
.ownerId(chatRoom.getFoodTruck().getOwner().getUserId())
.lastMessage(null)
.lastMessageSendTime(null)
.sortKey(Long.MAX_VALUE - chatRoom.getChatRoomId()) // 수정
.build();
}이렇게 하면 신규 채팅방도 첫 페이지에 정상적으로 표시되고, 이후 메시지가 도착하면 updateLastMessage()에서 타임스탬프 기반 sortKey로 업데이트되어 정렬 순서가 자연스럽게 조정됩니다. 화이팅! 🚀
...java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepositoryImpl.java
Show resolved
Hide resolved
| public record SendChatMessageRequest( | ||
| @Schema(description = "메시지 내용", example = "안녕하세요!") | ||
| String content, | ||
| @Schema(description = "메시지 타입 (TEXT or IMAGE)", example = "TEXT") | ||
| MessageContentType contentType | ||
| ) { | ||
| } |
There was a problem hiding this comment.
필수 검증 추가 필요
content 필드와 contentType 필드에 검증 애노테이션이 누락되어 있습니다. 빈 메시지나 null 값이 전송되는 것을 방지하기 위해 검증이 필요합니다.
다음 diff를 적용하여 검증을 추가하세요:
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Size;
+
public record SendChatMessageRequest(
@Schema(description = "메시지 내용", example = "안녕하세요!")
+ @NotBlank(message = "메시지 내용은 필수입니다.")
+ @Size(max = 1000, message = "메시지는 1000자를 초과할 수 없습니다.")
String content,
@Schema(description = "메시지 타입 (TEXT or IMAGE)", example = "TEXT")
+ @NotNull(message = "메시지 타입은 필수입니다.")
MessageContentType contentType
) {
}🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/chat/presentation/dto/request/SendChatMessageRequest.java
around lines 6 to 12, add validation annotations to the record components:
annotate content with @NotBlank (to prevent null/empty/whitespace messages) and
annotate contentType with @NotNull (to prevent null message type), and import
the corresponding javax.validation.constraints classes; ensure the controller
endpoint enforces validation (e.g., @Valid on the request parameter) so the
constraints are applied at runtime.
| public static ChatRoomResponse from(ChatRoom chatRoom, ChatRoomMetaDataProjection meta, boolean isOwner) { | ||
| // 현재 뷰 기준 상대방 정보 | ||
| User oppenent = isOwner ? chatRoom.getMember() : chatRoom.getFoodTruck().getOwner(); | ||
| String name = oppenent.getName(); | ||
|
|
||
| String foodTruckName = chatRoom.getFoodTruck().getFoodTruckInfo().getName(); | ||
| String profileImageUrl = oppenent.getProfileImageUrl(); | ||
|
|
||
| String lastMessage = meta.getLastMessage(); | ||
| String lastMessageSendTime = (meta.getLastMessageSendTime() != null) | ||
| ? DateUtil.formatLocalDateTime(meta.getLastMessageSendTime()) | ||
| : null; | ||
|
|
||
| long unreadCount = Optional.ofNullable(meta.getUnreadCount()).orElse(0L); | ||
|
|
||
| return new ChatRoomResponse( | ||
| chatRoom.getChatRoomId(), | ||
| name, | ||
| foodTruckName, | ||
| profileImageUrl, | ||
| lastMessage, | ||
| lastMessageSendTime, | ||
| unreadCount | ||
| ); |
There was a problem hiding this comment.
일반 유저 관점에서 foodTruckName 노출 버그
스키마 설명에 따르면 일반 유저가 목록을 볼 때 foodTruckName은 null이어야 합니다. 그러나 현재 구현은 isOwner 여부와 관계없이 항상 푸드트럭 이름을 채워 반환합니다. 일반 유저 뷰에서 예기치 않게 푸드트럭명이 노출되는 UX 버그이므로, isOwner가 false인 경우에는 null을 내려주도록 조건 분기해 주세요.
예시 수정:
- String foodTruckName = chatRoom.getFoodTruck().getFoodTruckInfo().getName();
+ String foodTruckName = isOwner
+ ? chatRoom.getFoodTruck().getFoodTruckInfo().getName()
+ : null;📝 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 static ChatRoomResponse from(ChatRoom chatRoom, ChatRoomMetaDataProjection meta, boolean isOwner) { | |
| // 현재 뷰 기준 상대방 정보 | |
| User oppenent = isOwner ? chatRoom.getMember() : chatRoom.getFoodTruck().getOwner(); | |
| String name = oppenent.getName(); | |
| String foodTruckName = chatRoom.getFoodTruck().getFoodTruckInfo().getName(); | |
| String profileImageUrl = oppenent.getProfileImageUrl(); | |
| String lastMessage = meta.getLastMessage(); | |
| String lastMessageSendTime = (meta.getLastMessageSendTime() != null) | |
| ? DateUtil.formatLocalDateTime(meta.getLastMessageSendTime()) | |
| : null; | |
| long unreadCount = Optional.ofNullable(meta.getUnreadCount()).orElse(0L); | |
| return new ChatRoomResponse( | |
| chatRoom.getChatRoomId(), | |
| name, | |
| foodTruckName, | |
| profileImageUrl, | |
| lastMessage, | |
| lastMessageSendTime, | |
| unreadCount | |
| ); | |
| public static ChatRoomResponse from(ChatRoom chatRoom, ChatRoomMetaDataProjection meta, boolean isOwner) { | |
| // 현재 뷰 기준 상대방 정보 | |
| User oppenent = isOwner ? chatRoom.getMember() : chatRoom.getFoodTruck().getOwner(); | |
| String name = oppenent.getName(); | |
| String foodTruckName = isOwner | |
| ? chatRoom.getFoodTruck().getFoodTruckInfo().getName() | |
| : null; | |
| String profileImageUrl = oppenent.getProfileImageUrl(); | |
| String lastMessage = meta.getLastMessage(); | |
| String lastMessageSendTime = (meta.getLastMessageSendTime() != null) | |
| ? DateUtil.formatLocalDateTime(meta.getLastMessageSendTime()) | |
| : null; | |
| long unreadCount = Optional.ofNullable(meta.getUnreadCount()).orElse(0L); | |
| return new ChatRoomResponse( | |
| chatRoom.getChatRoomId(), | |
| name, | |
| foodTruckName, | |
| profileImageUrl, | |
| lastMessage, | |
| lastMessageSendTime, | |
| unreadCount | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomResponse.java
around lines 27 to 50, the code always sets foodTruckName regardless of viewer
role; change the assignment so that foodTruckName is populated only when isOwner
is true and set to null when isOwner is false (i.e., conditionally assign
foodTruckName = isOwner ? chatRoom.getFoodTruck().getFoodTruckInfo().getName() :
null) before constructing the ChatRoomResponse.
| throw new AuthException(ErrorCode.AUTH_TOKEN_NOT_FOUND); | ||
| } | ||
|
|
||
| String token = authorization.split(" ")[1]; |
There was a problem hiding this comment.
배열 인덱스 접근 전 경계 검사가 필요합니다.
authorization.split(" ")의 결과 배열 길이를 확인하지 않고 [1]에 접근하면 ArrayIndexOutOfBoundsException이 발생할 수 있습니다. Authorization 헤더 형식이 잘못된 경우(예: 공백이 없거나 토큰 값이 없는 경우) 런타임 오류가 발생합니다.
다음 diff를 적용하여 안전하게 토큰을 추출하세요:
- String token = authorization.split(" ")[1];
+ String[] parts = authorization.split(" ");
+ if (parts.length != 2) {
+ throw new AuthException(ErrorCode.AUTH_INVALID_TOKEN);
+ }
+ String token = parts[1];📝 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 token = authorization.split(" ")[1]; | |
| String[] parts = authorization.split(" "); | |
| if (parts.length != 2) { | |
| throw new AuthException(ErrorCode.AUTH_INVALID_TOKEN); | |
| } | |
| String token = parts[1]; |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/global/common/security/interceptor/StompAuthChannelInterceptor.java
around line 36, the code directly accesses authorization.split(" ")[1] which can
throw ArrayIndexOutOfBoundsException when the Authorization header is null,
malformed, or missing a token; update the logic to first null-check/trim the
authorization header, either check authorization.startsWith("Bearer ") and take
substring after the scheme or split with a limit of 2 and verify parts.length >
1 and that parts[1] is not blank, and if the header is invalid respond by
logging and rejecting the message/throwing the appropriate authentication
exception instead of blindly indexing the array.
| import java.time.Duration; | ||
| import java.time.LocalDateTime; | ||
|
|
||
| public class DateUtil { | ||
| // LocalDateTime을 "오후 hh:mm" 형식의 문자열로 변환하는 메서드 (하루가 지난 경우 "어제" 이틀 이상이 지난 경우 "MM월 dd일" 1년 이상이 지난 경우 "yyyy년 MM월" 형식) | ||
| public static String formatLocalDateTime(LocalDateTime dateTime) { | ||
| LocalDateTime now = LocalDateTime.now(); | ||
| Duration duration = Duration.between(dateTime, now); | ||
|
|
||
| if (duration.toDays() < 1) { | ||
| // 오늘 | ||
| int hour = dateTime.getHour(); | ||
| String period = (hour >= 12) ? "오후" : "오전"; | ||
| hour = (hour > 12) ? hour - 12 : hour; | ||
| if (hour == 0) hour = 12; // 12시 처리 | ||
| int minute = dateTime.getMinute(); | ||
| return String.format("%s %d:%02d", period, hour, minute); | ||
| } else if (duration.toDays() < 2) { | ||
| // 어제 | ||
| return "어제"; | ||
| } else if (duration.toDays() < 365) { | ||
| // 올해 | ||
| return String.format("%d월 %d일", dateTime.getMonthValue(), dateTime.getDayOfMonth()); | ||
| } else { | ||
| // 1년 이상 | ||
| return String.format("%d년 %d월", dateTime.getYear(), dateTime.getMonthValue()); | ||
| } |
There was a problem hiding this comment.
날짜 구분 로직 개선 필요
Duration.toDays() 비교만으로 날짜를 판별하면 자정을 지난 지 24시간이 되지 않은 “어제” 메시지가 현재 날짜와 동일하게 취급되어 “오전/오후 시:분”으로 노출됩니다. 예: 2025년 11월 9일 23:50에 도착한 메시지를 2025년 11월 10일 00:10에 조회하면 사용자는 “어제”를 기대하지만 현재 구현은 “오전 11:50”을 반환합니다. 로컬 날짜 단위로 비교하도록 수정이 필요합니다.
다음과 같이 수정해 주세요:
-import java.time.Duration;
-import java.time.LocalDateTime;
+import java.time.Duration;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
@@
- if (duration.toDays() < 1) {
+ LocalDate today = now.toLocalDate();
+ LocalDate targetDate = dateTime.toLocalDate();
+
+ if (today.equals(targetDate)) {
@@
- } else if (duration.toDays() < 2) {
+ } else if (today.minusDays(1).equals(targetDate)) {
@@
- } else if (duration.toDays() < 365) {
+ } else if (today.getYear() == targetDate.getYear()) {🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/util/DateUtil.java around lines 3
to 29, the current logic uses Duration.toDays() to decide "today/yesterday/this
year" which misclassifies messages around midnight; change the comparisons to
use LocalDate-based checks: compute LocalDate nowDate = now.toLocalDate() and
LocalDate msgDate = dateTime.toLocalDate(), then if msgDate.isEqual(nowDate)
format as "오전/오후 h:mm", else if msgDate.equals(nowDate.minusDays(1)) return
"어제", else if msgDate.getYear() == nowDate.getYear() return "M월 d일", else return
"yyyy년 M월". Ensure hour conversion to 12-hour and minute padding remain the
same.
ksg1227
left a comment
ksg1227
left a comment
There was a problem hiding this comment.
확인했습니다!
처음 보는 내용이기도 하고 익숙하지 않은 내용도 많아서 쉽지 않네요..
일단 읽음 처리에 대해서는 현준님이 저보다 더 고민 많이 하시고 찾아보셨겠지만, 제가 처음에 들었던 생각은
-
가장 최근의 채팅부터 확인해서 최초로 read = true 인 메세지를 찾음. 그렇다면 그 이전의 메세지들은 어차피 전부 다 read = true 상태일거기 때문에 스캔할 필요가 없음. 그럼 이렇게 최초로 read=true 인 메세지를 찾아서 해당 메세지의 id 를 확인하고, 그보다 더 큰 id, 즉 그 이후에 생성된 메세지들을 다 read = true 로 바꿈.
이렇게 쿼리 2번으로 처리하기도 가능할 것 같긴 한데.. 일종의 쇼트서킷처럼 동작하도록?
메세지 수가 적을 때에는 비효율적이겠지만 메세지가 굉장히 많이 쌓인 방이라면 나쁘지 않을지도?
라는 생각이 처음에 들었었고, -
ChatRoomMetaData 에 member의 마지막 접속 시간, owner 의 마지막 접속 시간도 함께 저장해두었다가, 그 시간을 확인해서 그 시간 이후의 메세지들에 대해서만 읽음 처리를 하는 것도 가능하긴 하지 않을까?
일단 이런 방안들이 떠오르긴 하는데 가능할지 딱 판단이 서진 않네요..
현준님이 말씀하신 방법에 대한 생각은, 우선 Subscribe, Unsubscribe 커맨드를 핸들링 함으로써 실제로 채팅방에 마지막 접속한 시간을 알아낼 수 있는가? 가 궁금합니다.
사용자가 마지막으로 채팅방에 접속한 시간을 알아야 그 이후에 접속했을 때 마지막으로 접속한 시간 이후의 메세지들만 read 처리할 수 있을테니까요
추가적으로, 이벤트 처리를 도입한다면, 읽음 처리를 비동기적으로 수행하시겠다는 의도이신건지 궁금해요. 채팅방에 접속한 시점에 읽음 처리를 수행하도록 하는 이벤트를 발행하고 리스너에서 처리하는 식으로?
또, 현재 ChatRoom과 Reservation의 연관관계가 맺어져있지 않았는데 저희 서비스에서는 Reservation은 ChatRoom과 1:1 대응 되어야 하는 것으로 파악했습니다. 따라서, Reservation에 ChatRoom을 OneToOne으로 걸고 예약 생성 API시에 프론트로부터 ChatRoomId를 받은다음 주입해줄 생각인데 이에 대한 상균님 의견도 궁금합니다.!!
-> 좋습니다!!!
우선 1차적으로 코멘트 남기겠습니다!
| @Table(name = "chat_messages") | ||
| @Getter | ||
| @Document(collection = "chat_messages") | ||
| @CompoundIndex(name = "room_time_idx", def = "{'roomId': 1, 'sendTime': 1}") |
There was a problem hiding this comment.
오호 인덱스까지 생성하셨군요 👍🏻 몽고DB도 인덱스 설정이 가능한 게 신기하네요.
| def = "{ 'ownerId': 1, 'sortKey': -1 }" | ||
| ) | ||
| }) | ||
| public class ChatRoomMetaData { |
There was a problem hiding this comment.
이 ChatRoomMetaData 를 도입하신 이유 확인했습니다!
한 가지 궁금한 점은, 이 클래스는 오직 채팅방 목록에서 가장 최근의 메세지, 메세지 전송 시간 등을 확인하기 위한 용도라고 생각하면 되는걸까요?
그 이외에 실제 채팅방에 들어가서 채팅 내역을 조회할 때에는 따로 ChatMessageMetaData 를 참조할 필요가 없는 것일까요?
There was a problem hiding this comment.
네넵 현재 ChatRoomMetaData는 2가지 용도로 사용중입니다.
- 채팅방 목록 조회시에 메시지 전송 시간과 최근 메시지 조회
- 채팅 전송시에 유저가 해당 채팅방에 채팅을 보낼 권한이 있는지 검증
추가적으로 PR에서 언급드린 것처럼 이벤트를 도입해서 접속 시간을 체크한다면, ChatRoomMetaData는 사장님과 예약자의 마지막 채팅방 접속 시간을 저장하고 읽음 표시 API에서 이를 활용할 수 있을 것 같습니다.
| if(isOwner && user.getRole() != Role.OWNER) { | ||
| throw new BusinessException(USER_FORBIDDEN); | ||
| } |
There was a problem hiding this comment.
여기서 isOwner 는 '차콜 서비스 자체에서 사장님으로 등록되어있는지' 를 의미하는 게 아니라, 채팅방 내의 예약자 <-> 사장님 관계에서 예약중인 푸드트럭의 사장님인지 여부를 의미하는 거라고 생각하면 되죠?
다른 푸드트럭의 사장님도 예약자 신분이 될 수 있다보니까 이런 식으로 구분해야 뷰에 차이를 둘 수 있긴 하겠네요 좋슴다
There was a problem hiding this comment.
네넵 초기에 isOwner라는 플래그를 프론트로부터 받지 않고 구현을 하니 사장님과 사장님간의 채팅시에 논리적인 모순이 생기더라구요. 따라서, isOwner는 말씀하신대로 현재 예약자와 사장님의 관계를 구별해주기 위한 용도라고 생각하시면 될 것 같습니다. 프론트 쪽은 사장님과 예약자 뷰가 명확하게 나뉘어져 있으니 이를 보내주는 것이 쉬울거라고 판단했습니다!
| private Long memberId; | ||
| private Long ownerId; |
There was a problem hiding this comment.
ChatRoom 자체에 Member 란 FoodTruck 정보가 있긴 해서 chatRoomMetaData 내에도 memberId, ownerId 가 필요한 이유가 정확히 무엇인지 궁금합니다!
validateParticipant() 메서드를 통해 추측해보자면 실제로 해당 채팅방에 속한 유저나 사장님이 아니라면 메타데이터 조회 시 에러가 터지도록 하기 위함인 건가요?
There was a problem hiding this comment.
네넵 정확합니다! 초기에 ChatRoomMetaData없이 구현을 해보니 채팅 전송마다 채팅방 권한을 확인하기 위해 RDB를 조회해야 했는데 이러면 MongoDB를 사용하는 이점이 전혀 없다고 판단했습니다!
| @Aggregation(pipeline = { | ||
| // 1. 내가 속한 채팅방만 필터링 (isOwner 기준) | ||
| "{ '$match': { '$expr': { '$and': [" + | ||
| " { '$cond': [ ?1, { '$eq': ['$ownerId', ?0] }, { '$eq': ['$memberId', ?0] } ] }," + | ||
| " { '$lt': ['$sortKey', ?2] }" + | ||
| "] } } }", | ||
|
|
||
| // 2. 안읽은 메시지 개수 계산 (chat_messages 컬렉션 join) | ||
| "{ '$lookup': { " + |
There was a problem hiding this comment.
오 몽고DB를 안써봤다보니 이런 Aggregation 은 처음보는데 문법은 아예 다르지만 기능이 SQL과 상당히 유사하네요 신기방기
| public interface ChatRoomMetaDataProjection { | ||
| Long getRoomId(); | ||
|
|
||
| Long getMemberId(); | ||
| Long getOwnerId(); | ||
|
|
||
| String getLastMessage(); | ||
| LocalDateTime getLastMessageSendTime(); | ||
|
|
||
| Long getSortKey(); | ||
|
|
||
| Long getUnreadCount(); | ||
| } |
There was a problem hiding this comment.
Projection 활용 굿입니다.
아무래도 ChatRoomMetaData 자체에는 unreadCount 자체를 저장하지 않다보니 Aggregation 을 통해 조회해올 때 이 Projection을 사용하는 것이라고 이해하였는데 제가 제대로 이해한 게 맞을까유
There was a problem hiding this comment.
네넵 unreadCount도 필드로 저장해둘까도 생각했지만, 너무 많은 반정규화라는 생각이 들어서 우선은 도입하지 않았습니다!
| Query query = new Query(); | ||
| query.addCriteria(Criteria.where("roomId").is(roomId)); | ||
| // 내가 보낸 메시지는 읽음 처리 대상이 아님 | ||
| query.addCriteria(Criteria.where("senderId").ne(userId)); | ||
| // 아직 읽지 않은 메시지만 | ||
| query.addCriteria(Criteria.where("read").is(false)); | ||
|
|
||
| Update update = new Update(); | ||
| update.set("read", true); | ||
|
|
||
| // 조건에 맞는 모든 문서를 한 번에 업데이트 | ||
| mongoTemplate.updateMulti(query, update, ChatMessage.class); |
There was a problem hiding this comment.
여기는 뭔가 QueryDsl 이나 Criteria API 같은 걸 사용하신 줄 알았는데, MongoDb 에서 지원하는 기술이 따로 있군여 이것도 신기하네요.
몽고 DB 생각보다 되게 편해보이네요.
그리고 이렇게 계속해서 업데이트 하는 과정들이 있다보니까 확실히 성능을 챙기려면 RDB 보단 NoSQL 을 활용하는 게 좋을 것 같다는 생각이 갈수록 드네요.
There was a problem hiding this comment.
DB 2개 스택을 사용하는 것이 익숙하지 않아서 조금 복잡하긴 하지만, 그래도 성능적으로는 확실히 좋을 것 같다는 생각이 드네요 👍🏻
| @Override | ||
| public Message<?> preSend(Message<?> message, MessageChannel channel) { | ||
| StompHeaderAccessor accessor = | ||
| MessageHeaderAccessor.getAccessor(message, StompHeaderAccessor.class); | ||
|
|
||
| // CONNECT 요청에 대해서 인증 처리 & JWT 토큰 검증 | ||
| if (StompCommand.CONNECT.equals(accessor.getCommand())) { | ||
| String authorization = accessor.getFirstNativeHeader(JWT_HEADER_KEY.getValue()); | ||
| if (authorization == null || !authorization.startsWith(JWT_PREFIX.getValue())) { | ||
| throw new AuthException(ErrorCode.AUTH_TOKEN_NOT_FOUND); | ||
| } |
상세한 리뷰 감사합니다~! 우선 생각하신 2가지 방안 중에 저는 두번째 방안을 생각하긴 했습니다. 그런데 상균님이 제시하신 1번방식도 생각해보니 간단하면서도 2번과 성능이 크게 다를 것 같지는 않을 것 같기도..? 하네요.
저는 이벤트를 도입한다면 읽음 처리가 아닌 접속 시간을 저장하는 것을 비동기적으로 수행할 생각이였습니다. unsubscribe시에 사용자가 채팅방을 나갔다는 이벤트를 발행하고 이벤트 리스터가 해당 채팅방의 마지막 사용자 접속 시간을 저장 -> 나중에 사용자가 읽음 표시 API 요청을 보냈을때 마지막 접속 시간 이후에 온 메시지만 모두 읽음 처리 흐름으로 구상하긴 했습니다. 흐음 첫번째와 두번째 방식 중에 트레이드오프를 좀 따져봐야될 것 같네요,, |
오호 접속 시간 저장을 비동기적으로 수행하는 거였군여. |
@ksg1227 음 근데 조금 고민해보니, 구현 난이도를 생각했을 때 상균님이 말씀하신 단순 isRead 플래그로 조회 + 배치 업데이트를 하는 것이 훨씬 단순할 것 같다는 생각이 드네요. 그렇게 한번 구현해보겠습니다. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java (1)
5-8: 사용되지 않는 import 제거 권장
Reservation,DomainRuleException,ErrorCodeimport가 이 파일에서 사용되지 않습니다.import konkuk.chacall.domain.foodtruck.domain.model.FoodTruck; -import konkuk.chacall.domain.reservation.domain.model.Reservation; import konkuk.chacall.domain.user.domain.model.User; -import konkuk.chacall.global.common.exception.DomainRuleException; -import konkuk.chacall.global.common.exception.code.ErrorCode; import lombok.*;src/main/java/konkuk/chacall/domain/chat/domain/ChatRoomMetaData.java (1)
65-71: 타임존 처리 일관성 검토 필요
ZoneId.systemDefault()는 서버 환경에 따라 다른 값을 반환할 수 있어 분산 시스템에서 정렬 순서가 불일치할 수 있습니다. UTC 또는 고정된 타임존 사용을 권장합니다.+import java.time.ZoneOffset; + public void updateLastMessage(String content, LocalDateTime sendTime) { this.lastMessage = content; this.lastMessageSendTime = sendTime; if (sendTime != null) { - this.sortKey = sendTime.atZone(ZoneId.systemDefault()).toInstant().toEpochMilli(); + this.sortKey = sendTime.toInstant(ZoneOffset.UTC).toEpochMilli(); } }참고:
lastMessageSendTime이 UTC 기준으로 저장된다면 위 수정이 적합합니다. 만약 KST 등 특정 타임존 기준이라면ZoneId.of("Asia/Seoul")사용을 고려하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java(2 hunks)src/main/java/konkuk/chacall/domain/chat/domain/ChatRoomMetaData.java(1 hunks)src/main/java/konkuk/chacall/domain/reservation/application/info/ReservationInfoService.java(3 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java(4 hunks)src/main/java/konkuk/chacall/domain/reservation/presentation/dto/request/CreateReservationRequest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-22T08:13:37.857Z
Learnt from: buzz0331
Repo: CHA-CALL/ChaCall-Server PR: 30
File: src/main/java/konkuk/chacall/domain/reservation/application/ReservationService.java:52-68
Timestamp: 2025-09-22T08:13:37.857Z
Learning: ChaCall 프로젝트에서 OWNER 역할은 MEMBER 권한을 포함합니다. memberValidator.validateAndGetMember()는 MEMBER와 OWNER 모두를 허용하므로, OWNER가 예약 취소 등 MEMBER 기능을 사용할 수 있습니다.
Applied to files:
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoomMetaData.java
🧬 Code graph analysis (2)
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java (3)
src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (1)
Getter(18-182)src/main/java/konkuk/chacall/domain/chat/domain/ChatRoomMetaData.java (1)
Getter(15-79)src/main/java/konkuk/chacall/domain/user/domain/model/User.java (1)
Getter(9-86)
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoomMetaData.java (1)
src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationDateList.java (1)
AllArgsConstructor(16-81)
🔇 Additional comments (6)
src/main/java/konkuk/chacall/domain/reservation/application/info/ReservationInfoService.java (1)
31-47: LGTM!ChatRoom 조회 및 검증 로직이 적절하게 추가되었습니다. 예약 생성 시 ChatRoom 연관관계가 올바르게 설정됩니다.
src/main/java/konkuk/chacall/domain/reservation/presentation/dto/request/CreateReservationRequest.java (1)
13-16: LGTM!chatRoomId 필드가 적절한 검증 어노테이션과 함께 추가되었습니다.
src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (1)
51-55: LGTM!ChatRoom과의 1:1 연관관계가 적절하게 설정되었습니다. 주석에 명시된 대로 기존 데이터 마이그레이션 후
nullable = false로 변경하는 것이 좋겠습니다.src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java (1)
32-37: LGTM!팩토리 메서드가 깔끔하게 구현되었습니다. member와 foodTruck을 받아 ChatRoom을 생성하는 패턴이 일관성 있습니다.
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoomMetaData.java (2)
20-31: LGTM!member 및 owner 기준 조회를 위한 복합 인덱스가 적절하게 정의되었습니다.
sortKey내림차순으로 최근 메시지 기준 정렬이 효율적으로 지원됩니다.
54-63: LGTM!
Long.MAX_VALUE - chatRoomId를 초기 sortKey로 사용하여 메시지가 없는 채팅방을 목록 하단으로 배치하는 전략이 적절합니다.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/konkuk/chacall/domain/chat/presentation/ChatRestController.java (2)
67-83: getChatMessages 엔드포인트 ExceptionDescription 누락다른 엔드포인트들은
@ExceptionDescription을 사용하여 예외 응답을 문서화하고 있지만, 이 엔드포인트에는 누락되어 있습니다. Swagger 문서의 일관성을 위해 추가를 고려해 주세요.
99-109: markMessagesAsRead 엔드포인트 ExceptionDescription 누락마찬가지로
@ExceptionDescription이 누락되어 있습니다. 채팅방 존재 여부 검증 등 발생 가능한 예외에 대한 문서화를 추가해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/konkuk/chacall/domain/chat/application/ChatService.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/ChatRestController.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomMetaDataResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java(2 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/chacall/domain/chat/application/ChatService.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-22T08:13:37.857Z
Learnt from: buzz0331
Repo: CHA-CALL/ChaCall-Server PR: 30
File: src/main/java/konkuk/chacall/domain/reservation/application/ReservationService.java:52-68
Timestamp: 2025-09-22T08:13:37.857Z
Learning: ChaCall 프로젝트에서 OWNER 역할은 MEMBER 권한을 포함합니다. memberValidator.validateAndGetMember()는 MEMBER와 OWNER 모두를 허용하므로, OWNER가 예약 취소 등 MEMBER 기능을 사용할 수 있습니다.
Applied to files:
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java
🧬 Code graph analysis (2)
src/main/java/konkuk/chacall/domain/chat/presentation/ChatRestController.java (1)
src/main/java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepositoryImpl.java (1)
RequiredArgsConstructor(10-29)
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java (2)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/chat/application/ChatService.java (1)
Service(22-71)
🔇 Additional comments (6)
src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java (1)
3-3: The 1:1 mapping and unique constraint are correctly configured; fetch optimization suggestion is unnecessary for current usage.The review comment's concerns about the Reservation–ChatRoom relationship design have been verified:
✅ 1:1 mapping confirmed: Line 52-54 of Reservation.java shows
@OneToOne(fetch = FetchType.LAZY)with@JoinColumn(name = "chat_room_id", unique = true). The unique constraint on the database column is explicitly set.✅ Design premise validated: The mapping correctly enforces a one-to-one relationship at both JPA and database levels.
⚠️ Fetch optimization not needed: The only usage offindByChatRoom(in ChatRoomService.java, line 62) only extracts thereservationIdvia.map(Reservation::getReservationId). It does not accessmember,foodTruck, orownerfields, so there is no N+1 or LAZY initialization risk. A fetch-optimized variant would be premature.ℹ️ Future nullable change: The entity includes a comment indicating
nullable = falseis planned (line 51), which is good for data consistency.The method signature and design are appropriate for the current usage pattern. No additional refactoring or fetch-join variant is required at this time.
src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java (1)
269-274: Swagger 문서와 실제 구현 불일치
CREATE_CHAT_ROOM에CHAT_ROOM_ALREADY_EXISTS에러 코드가 포함되어 있지만,ChatRoomService.createChatRoom구현에서는 기존 채팅방이 있을 경우 에러를 던지지 않고 기존 채팅방 ID를 반환합니다. Swagger 문서가 실제 동작과 일치하도록 해당 에러 코드를 제거하거나, 구현 의도에 따라 수정해 주세요.CREATE_CHAT_ROOM(new LinkedHashSet<>(Set.of( USER_NOT_FOUND, USER_FORBIDDEN, - FOOD_TRUCK_NOT_FOUND, - CHAT_ROOM_ALREADY_EXISTS + FOOD_TRUCK_NOT_FOUND ))),[inconsistent_summary, suggest_essential_refactor]
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java (3)
58-76: 참여자 검증 누락으로 정보 노출 가능이전 리뷰에서 지적된 사항입니다.
getChatRoomMetaData는 인증된user가 해당 채팅방의 실제 참여자(예약자 또는 푸드트럭 사장)인지 검증하지 않습니다. 임의의roomId로 호출하면 채팅 상대 이름과 푸드트럭 정보가 노출될 수 있습니다.public ChatRoomMetaDataResponse getChatRoomMetaData(User user, Long roomId, boolean isOwner) { ChatRoom chatRoom = chatRoomRepository.findById(roomId) .orElseThrow(() -> new EntityNotFoundException(CHAT_ROOM_NOT_FOUND)); + Long ownerId = chatRoom.getFoodTruck().getOwner().getUserId(); + Long memberId = chatRoom.getMember().getUserId(); + if (!ownerId.equals(user.getUserId()) && !memberId.equals(user.getUserId())) { + throw new BusinessException(CHAT_ROOM_FORBIDDEN); + } + Long reservationId = reservationRepository.findByChatRoom(chatRoom)[duplicate_comment, raise_critical_issue]
78-86: isOwner null 처리 필요이전 리뷰에서 지적된 사항입니다.
isOwner가Boolean래퍼 타입인데findChatRoomsForUser에 직접 전달하면 자동 언박싱 시NullPointerException이 발생합니다. 요청 파라미터가 비어 있는 경우를 안전하게 처리해 주세요.public CursorPagingResponse<ChatRoomResponse> getChatRooms(User member, Boolean isOwner, Long cursor, Integer size) { int pageSize = (size == null || size < 1) ? 20 : size; + boolean ownerView = Boolean.TRUE.equals(isOwner); Long cursorSortKey = (cursor != null) ? cursor : Long.MAX_VALUE; int limit = pageSize + 1; // 1. MongoDB에서 메타데이터 + unreadCount 조회 List<ChatRoomMetaDataProjection> metaList = - chatRoomMetaDataRepository.findChatRoomsForUser(member.getUserId(), isOwner, cursorSortKey, limit); + chatRoomMetaDataRepository.findChatRoomsForUser(member.getUserId(), ownerView, cursorSortKey, limit);그리고 Line 104의
ChatRoomResponse.from호출도 동일하게ownerView를 사용해야 합니다.
[duplicate_comment, raise_critical_issue]
39-56: createChatRoom 로직 적절함기존 채팅방이 있으면 재사용하고, MongoDB 메타데이터도 존재 여부를 확인 후 생성하는 방식이 적절합니다. 멱등성이 보장되어 프론트엔드에서 반복 호출해도 안전합니다.
src/main/java/konkuk/chacall/domain/chat/presentation/ChatRestController.java (1)
27-48: 컨트롤러 구조 적절함REST 컨트롤러가 표준적인 구조를 따르고 있으며, Swagger 문서화, 유효성 검증, 의존성 주입이 적절하게 설정되어 있습니다.
ChatService로 위임하는 단순 위임 패턴이 잘 적용되었습니다.
| List<ChatRoom> chatRooms = chatRoomRepository.findByChatRoomIdIn(roomIds); | ||
| Map<Long, ChatRoom> chatRoomMap = chatRooms.stream() | ||
| .collect(Collectors.toMap(ChatRoom::getChatRoomId, Function.identity())); | ||
|
|
||
| // 3. 메타데이터 + RDB 정보 조합해서 ChatRoomResponse 생성 | ||
| List<ChatRoomResponse> responses = metaList.stream() | ||
| .map(meta -> ChatRoomResponse.from(chatRoomMap.get(meta.getRoomId()), meta, isOwner)) | ||
| .toList(); |
There was a problem hiding this comment.
RDB 조회 결과 누락 시 NPE 가능성
chatRoomMap.get(meta.getRoomId())이 null을 반환할 경우(MongoDB와 RDB 간 데이터 불일치 시) ChatRoomResponse.from에서 NPE가 발생할 수 있습니다. 방어 로직을 추가하거나 null 필터링을 고려해 주세요.
List<ChatRoomResponse> responses = metaList.stream()
+ .filter(meta -> chatRoomMap.containsKey(meta.getRoomId()))
.map(meta -> ChatRoomResponse.from(chatRoomMap.get(meta.getRoomId()), meta, isOwner))
.toList();🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java
around lines 98 to 105, the code calls
ChatRoomResponse.from(chatRoomMap.get(meta.getRoomId()), ...) which can pass
null when the RDB lookup misses and cause an NPE; update the mapping to handle
missing ChatRoom entries by filtering out metas with null map values (or provide
a safe fallback) before calling ChatRoomResponse.from, and add a warn log entry
(including meta.getRoomId()) when a corresponding ChatRoom is not found so the
discrepancy between Mongo and RDB is visible.
| @Parameter(description = "현재 채팅방 기준 푸드트럭 사장인지 여부", example = "false") | ||
| @RequestParam final Boolean isOwner | ||
| ) { | ||
| return BaseResponse.ok( | ||
| chatService.getChatRoomMetaData(memberId, roomId, isOwner) | ||
| ); |
There was a problem hiding this comment.
isOwner null 시 NPE 발생 가능
isOwner가 Boolean 타입이지만 chatService.getChatRoomMetaData는 primitive boolean을 받습니다. 요청에서 isOwner 파라미터가 누락되면 자동 언박싱 시 NullPointerException이 발생합니다.
- @RequestParam final Boolean isOwner
+ @RequestParam(required = true) @NotNull(message = "isOwner 파라미터는 필수입니다.") final Boolean isOwner또는 서비스 레이어에서 Boolean 타입으로 받아 null-safe하게 처리하는 방법도 있습니다.
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/chat/presentation/ChatRestController.java
around lines 59-64, the controller accepts a boxed Boolean isOwner but passes it
to a service expecting primitive boolean, which can cause NPE if the request
omits the parameter; fix by making the request param primitive with a safe
default (e.g., @RequestParam(defaultValue = "false") final boolean isOwner) so
no null is possible, or alternatively keep Boolean and null-check/convert before
calling chatService.getChatRoomMetaData; update the controller accordingly.
...main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomMetaDataResponse.java
Outdated
Show resolved
Hide resolved
|
@ksg1227
update (
roomId = X
AND senderId != user
AND read = false
AND sendTime > lastReadMessage.sendTime
)
set read = true
update (
roomId = X
AND senderId != user
)
set read = true일단 구현해두었습니다! (아래 커밋) 보시고 피드백 부탁드릴게요~ |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepositoryImpl.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomMetaDataResponse.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomMetaDataResponse.java
🔇 Additional comments (1)
src/main/java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepositoryImpl.java (1)
1-13: LGTM! 클래스 구조가 깔끔합니다.MongoDB 커스텀 리포지토리 구현을 위한 표준 구조를 잘 따르고 있으며, 필요한 의존성(MongoTemplate)도 적절하게 주입되고 있습니다.
...java/konkuk/chacall/domain/chat/domain/repository/infra/ChatMessageCustomRepositoryImpl.java
Show resolved
Hide resolved
확인했습니다! 현재 구현 자체는 말씀하신 것이 잘 반영된 형태라고 판단됩니다. 그리고 로직도 꽤 깔끔하게 구현된 것 같아요. 인덱스만 잘 탄다면 성능도 괜찮지 않을까 생각합니다. 다만 코드 래빗 리뷰를 확인해보니까, senderId, read 여부까지 포함한 조회에서는 roomId 까지만 인덱스를 제대로 타고, 그 다음 조건들에 대해서는 인덱스를 제대로 타지 못해서 완전한 최적화는 아니라고 말하고있는 것 같아요. 그래서 @CompoundIndex(
name = "room_sender_read_time_idx",
def = "{'roomId': 1, 'senderId': 1, 'read': 1, 'sendTime': 1}"
)이런 복합 인덱스를 하나 추가하면 더 나을 것 같다고 언급하는 것 같아요! 한 번 확인해보면 좋을 것 같슴다. 구현은 굿굿입니다~ |
넵 좋은 것 같습니다. 기존 인덱스를 상균님이 말씀하신 인덱스로 교체하는게 좋을 것 같네요. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/konkuk/chacall/domain/chat/domain/ChatMessage.java (2)
5-21: Mongo 문서 매핑 및 복합 인덱스 구성이 의도와 잘 맞습니다
@Document+org.springframework.data.annotation.Id로 바꾸면서 예전에 지적되던 JPA@Id매핑 문제는 깔끔하게 정리된 것 같고,roomId, senderId, read, sendTime순서의@CompoundIndex도 설명해 주신 읽음 처리 쿼리(roomId + senderId + read 조건에서 sendTime 정렬/범위 조회) 패턴과 잘 맞습니다.다만 같은 컬렉션을 대상으로:
- 단순히
roomId기준 최신 메시지 1건 조회,- 또는
roomId기준sendTime페이징같이
senderId/read조건이 없는 쿼리가 있다면, 그런 패턴은(roomId, sendTime)전용 인덱스를 하나 더 두는 게 플랜 선택에 유리할 수 있으니, 실제 쿼리들을 한 번 훑어보시고 필요 여부만 검토해 보시면 좋겠습니다.
23-44: 필드 타입/팩토리 시그니처를 약간만 정리하면 가독성이 더 좋아질 것 같습니다현재 설계도 동작에는 문제 없어 보이지만, 몇 가지 일관성 관점에서 제안드립니다.
contentType필드는String인데,createChatMessage는MessageContentTypeEnum을 받아name()으로 변환하고 있습니다.
- 생성 시에는 타입 세이프해서 좋지만, 도메인 객체를 직접 다루는 쪽에서는 문자열 리터럴로 비교하게 되어 혼동 여지가 있습니다.
- 대안 1: 필드 타입도
MessageContentType으로 두고, Spring Data Mongo가 Enum을 문자열로 직렬화하게 맡깁니다.- 대안 2: 반대로 팩토리 메서드 시그니처를
String contentType으로 바꾸고, enum → string 변환은 상위 계층(서비스/컨트롤러)에서 끝내도록 맞춥니다.sendTime과read에@Builder.Default를 둔 것은createChatMessage경로에서 자동으로 현재 시각/읽지 않음으로 세팅되어 의도대로 잘 보입니다. 다만 장기적으로 여러 서버/타임존 환경을 고려하신다면:
LocalDateTime.now()대신 UTCInstant나OffsetDateTime기반으로 저장하거나,- Mongo Auditing의
@CreatedDate를 활용해 생성 시각을 일관되게 관리하는 쪽도 한 번쯤 고려해 볼 만합니다.지금 구조를 그대로 가져가셔도 기능상 문제는 없어 보이고, 위 내용은 주로 도메인 모델의 일관성과 향후 유지보수성을 위한 선택 사항에 가깝습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/konkuk/chacall/domain/chat/domain/ChatMessage.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/konkuk/chacall/domain/chat/domain/ChatMessage.java (1)
src/main/java/konkuk/chacall/domain/user/domain/model/User.java (1)
Getter(9-86)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomResponse.java (1)
30-55: 일반 유저 뷰에서foodTruckName노출 조건 분기 필요 (이전 리뷰 미반영)현재 구현은
isOwner여부와 관계없이 항상foodTruckName을 채워서 내려주고 있어, 일반 유저(예약자) 목록 뷰에서도 푸드트럭명이 노출됩니다. 기존 스키마/논의에 따르면 일반 유저 관점에서는 목록에서foodTruckName이null이어야 한다고 하셨으므로, 아래처럼isOwner기준으로 분기해 주는 게 안전해 보입니다.- User oppenent = isOwner ? chatRoom.getMember() : chatRoom.getFoodTruck().getOwner(); - String name = oppenent.getName(); - - String foodTruckName = chatRoom.getFoodTruck().getFoodTruckInfo().getName(); - String profileImageUrl = oppenent.getProfileImageUrl(); + User opponent = isOwner ? chatRoom.getMember() : chatRoom.getFoodTruck().getOwner(); + String name = opponent.getName(); + + String foodTruckName = isOwner + ? chatRoom.getFoodTruck().getFoodTruckInfo().getName() + : null; + String profileImageUrl = opponent.getProfileImageUrl();이렇게 하면:
- 사장님 뷰(
isOwner = true)에서는 상대(예약자) 이름 + 푸드트럭 이름이 모두 노출되고,- 일반 유저 뷰(
isOwner = false)에서는 상대(사장님) 이름과 프로필만 보이고foodTruckName은null로 유지됩니다.변수명 오타(
oppenent)도 위 예시처럼opponent로 정리해 두면 이후 읽기/검색 시 혼동을 줄일 수 있을 것 같습니다.src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java (3)
58-76: 채팅방 참여자 검증이 없어 다른 OWNER가 roomId만 알면 정보 열람 가능 (보안 취약점)
getChatRoomMetaData는 현재 다음만 체크합니다.
- roomId 로 ChatRoom 조회
isOwner && user.getRole() != Role.OWNER인 경우만USER_FORBIDDEN그러나 실제로는 해당 채팅방의 예약자/사장님이 아닌 사용자도, OWNER 권한만 있으면 roomId 를 알고 있을 때 상대 이름/푸드트럭 정보를 조회할 수 있습니다. 이는 이전 리뷰에서 지적된 내용이 아직 반영되지 않은 상태입니다.
isOwner플래그와 상관없이, 반드시 요청 사용자가 해당 채팅방의 참여자인지 검증해야 합니다. 예:ChatRoom chatRoom = chatRoomRepository.findById(roomId) .orElseThrow(() -> new EntityNotFoundException(CHAT_ROOM_NOT_FOUND)); + Long callerId = user.getUserId(); + Long ownerId = chatRoom.getFoodTruck().getOwner().getUserId(); + Long memberId = chatRoom.getMember().getUserId(); + + if (isOwner) { + // 사장님 뷰: caller 가 실제 이 방의 사장인지 확인 + if (!ownerId.equals(callerId)) { + throw new BusinessException(CHAT_ROOM_FORBIDDEN); + } + } else { + // 예약자 뷰: caller 가 실제 이 방의 예약자인지 확인 + if (!memberId.equals(callerId)) { + throw new BusinessException(CHAT_ROOM_FORBIDDEN); + } + } + - if(isOwner && user.getRole() != Role.OWNER) { - throw new BusinessException(USER_FORBIDDEN); - }
USER_FORBIDDEN대신CHAT_ROOM_FORBIDDEN등의 전용 에러 코드를 사용하면 원인 구분에도 도움이 됩니다.
78-87:Boolean isOwner파라미터의 null 처리 누락으로 NPE 가능 (이전 리뷰 미반영)
getChatRooms시그니처가Boolean isOwner이고, 그대로findChatRoomsForUser(..., isOwner, ...)및ChatRoomResponse.from(..., isOwner, ...)에 전달되고 있습니다.
요청에서isOwner를 생략하면null이 들어올 수 있고, 리포지토리 메서드/쿼리가boolean으로 언박싱하는 순간NullPointerException이 발생할 수 있습니다(이전에 지적되었던 부분).안전한 기본값(예:
null→ 예약자 뷰)을 정한 뒤, 전체 로직에서 primitive boolean 으로만 사용하도록 정규화하는 편이 좋습니다.- public CursorPagingResponse<ChatRoomResponse> getChatRooms(User member, Boolean isOwner, Long cursor, Integer size) { + public CursorPagingResponse<ChatRoomResponse> getChatRooms(User member, Boolean isOwner, Long cursor, Integer size) { int pageSize = (size == null || size < 1) ? 20 : size; Long cursorSortKey = (cursor != null) ? cursor : Long.MAX_VALUE; int limit = pageSize + 1; - // 1. MongoDB에서 메타데이터 + unreadCount 조회 - List<ChatRoomMetaDataProjection> metaList = - chatRoomMetaDataRepository.findChatRoomsForUser(member.getUserId(), isOwner, cursorSortKey, limit); + boolean ownerView = Boolean.TRUE.equals(isOwner); // null → false (예약자 뷰) + + // 1. MongoDB에서 메타데이터 + unreadCount 조회 + List<ChatRoomMetaDataProjection> metaList = + chatRoomMetaDataRepository.findChatRoomsForUser(member.getUserId(), ownerView, cursorSortKey, limit);이후
ChatRoomResponse.from호출 부분에서도isOwner대신ownerView를 넘기도록 맞춰 주시는 것이 일관성 측면에서 좋습니다(아래 다른 코멘트에서 함께 제안).
93-109: RDB 조회 누락 시 NPE 가능 + 예약 확정 여부 Map 변환 로직 보완 필요
chatRoomMap.get(meta.getRoomId())가 null 일 수 있음 (이전 리뷰 미반영)
MongoDB 메타데이터와 RDBChatRoom이 불일치하는 경우(예: RDB에서 방 삭제, Mongo에는 잔존),findByChatRoomIdIn결과에 없는 roomId가 metaList에 남을 수 있습니다. 이때:ChatRoom chatRoom = chatRoomMap.get(meta.getRoomId()); ChatRoomResponse.from(chatRoom, meta, ...);에서
chatRoom이 null 이면 곧바로 NPE 가 발생합니다.최소한 null 인 메타는 필터링해 주는 편이 안전합니다. 예:
- List<ChatRoomResponse> responses = metaList.stream() - .map(meta -> ChatRoomResponse.from(chatRoomMap.get(meta.getRoomId()), meta, isOwner, - reservationConfirmedMap.getOrDefault(meta.getRoomId(), false))) - .toList(); + List<ChatRoomResponse> responses = metaList.stream() + .filter(meta -> chatRoomMap.containsKey(meta.getRoomId())) + .map(meta -> ChatRoomResponse.from( + chatRoomMap.get(meta.getRoomId()), + meta, + ownerView, + reservationConfirmedMap.getOrDefault(meta.getRoomId(), false) + )) + .toList();(가능하다면
@Slf4j추가 후 누락 roomId에 대해 warn 로그를 남겨 두면 운영 시 데이터 불일치 탐지에 도움이 됩니다.)
- 예약 확정 여부 조회: 리포지토리 리턴 타입 변경 반영
앞선ReservationRepository코멘트에서 제안한 대로List<ReservationConfirmedProjection>를 반환하도록 변경했다면, 여기서는 다음처럼 Map 으로 변환하는 단계가 필요합니다.Map<Long, Boolean> reservationConfirmedMap = reservationRepository.findReservationConfirmedByChatRoomIds(roomIds).stream() .collect(Collectors.toMap( ReservationConfirmedProjection::getRoomId, ReservationConfirmedProjection::isConfirmed ));이후
getOrDefault사용 방식은 그대로 유지할 수 있습니다.
🧹 Nitpick comments (1)
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java (1)
39-56:createChatRoom의 1:1 설계를 DB 레벨 제약으로 보강하는 것이 좋습니다
member + foodTruck조합으로findByMemberAndFoodTruck후 없으면 새로 생성하는 패턴이라, 논리적으로는 1:1 관계를 의도하고 있지만, 동시에 두 요청이 들어오는 경우 중복 채팅방이 생성될 수 있습니다. (두 트랜잭션 모두findByMemberAndFoodTruck에서 비어 있는 상태를 보고 각각save호출)Reservation–ChatRoom 을 1:1로 보시려면:
ChatRoom테이블에(member_id, food_truck_id)유니크 인덱스를 추가하고,- 중복 insert 시
DataIntegrityViolationException등을 캐치해 재조회하는 패턴으로 보완하는 것을 추천드립니다. 설계 상 이미 유니크 제약을 넣어두셨다면, 해당 제약에 대한 주석/테스트를 남겨 두면 향후 유지보수에 도움이 될 것 같습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java(1 hunks)src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: buzz0331
Repo: CHA-CALL/ChaCall-Server PR: 13
File: src/main/java/konkuk/chacall/domain/user/presentation/dto/request/UpdateUserInfoRequest.java:8-31
Timestamp: 2025-09-16T06:26:53.978Z
Learning: ChaCall-Server 프로젝트에서는 수정 작업에 PATCH 메소드를 사용하는 것이 팀 컨벤션으로 정해져 있다.
📚 Learning: 2025-09-22T08:13:37.857Z
Learnt from: buzz0331
Repo: CHA-CALL/ChaCall-Server PR: 30
File: src/main/java/konkuk/chacall/domain/reservation/application/ReservationService.java:52-68
Timestamp: 2025-09-22T08:13:37.857Z
Learning: ChaCall 프로젝트에서 OWNER 역할은 MEMBER 권한을 포함합니다. memberValidator.validateAndGetMember()는 MEMBER와 OWNER 모두를 허용하므로, OWNER가 예약 취소 등 MEMBER 기능을 사용할 수 있습니다.
Applied to files:
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java
🧬 Code graph analysis (2)
src/main/java/konkuk/chacall/domain/chat/presentation/dto/response/ChatRoomResponse.java (1)
src/main/java/konkuk/chacall/global/common/util/DateUtil.java (1)
DateUtil(6-32)
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java (2)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/chat/application/ChatService.java (1)
Service(22-71)
| Optional<Reservation> findByChatRoom(ChatRoom chatRoom); | ||
|
|
||
| @Query("SELECT r.chatRoom.chatRoomId AS roomId, " + | ||
| "CASE WHEN r.reservationStatus = 'CONFIRMED' THEN true ELSE false END AS confirmed " + | ||
| "FROM Reservation r " + | ||
| "WHERE r.chatRoom.chatRoomId IN :roomIds") | ||
| Map<Long, Boolean> findReservationConfirmedByChatRoomIds(List<Long> roomIds); |
There was a problem hiding this comment.
Map<Long, Boolean> 리턴 타입은 Spring Data JPA에서 직접 지원되지 않아 런타임 오류 위험이 있습니다
현재 JPQL @query 메서드를 Map<Long, Boolean> 으로 선언하면, Spring Data JPA가 결과를 어떻게 매핑해야 할지 몰라 런타임에 예외가 발생할 가능성이 큽니다. 일반적으로는 다음 두 패턴 중 하나를 사용합니다.
- 프로젝션 인터페이스 사용 (권장)
별도 프로젝션 인터페이스를 정의한 뒤, 서비스 레이어에서 Map으로 변환:
// 예: 새 인터페이스 정의
public interface ReservationConfirmedProjection {
Long getRoomId();
boolean isConfirmed();
}- @Query("SELECT r.chatRoom.chatRoomId AS roomId, " +
- "CASE WHEN r.reservationStatus = 'CONFIRMED' THEN true ELSE false END AS confirmed " +
- "FROM Reservation r " +
- "WHERE r.chatRoom.chatRoomId IN :roomIds")
- Map<Long, Boolean> findReservationConfirmedByChatRoomIds(List<Long> roomIds);
+ @Query("SELECT r.chatRoom.chatRoomId AS roomId, " +
+ "CASE WHEN r.reservationStatus = 'CONFIRMED' THEN true ELSE false END AS confirmed " +
+ "FROM Reservation r " +
+ "WHERE r.chatRoom.chatRoomId IN :roomIds")
+ List<ReservationConfirmedProjection> findReservationConfirmedByChatRoomIds(
+ @Param("roomIds") List<Long> roomIds);그리고 ChatRoomService 쪽에서는 다음처럼 변환:
Map<Long, Boolean> reservationConfirmedMap =
reservationRepository.findReservationConfirmedByChatRoomIds(roomIds).stream()
.collect(Collectors.toMap(
ReservationConfirmedProjection::getRoomId,
ReservationConfirmedProjection::isConfirmed
));List<Object[]>사용 후 수동 매핑
타입 안정성이 떨어져 가급적 1번 방식을 추천합니다.
또한 :roomIds 바인딩을 위해 @Param("roomIds") 를 명시해 주는 편이 안전합니다(현재도 컴파일 옵션에 따라 동작할 수 있지만, 명시적으로 맞춰 두는 것이 유지보수에 유리합니다).
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java
around lines 55-61, the repository method returning Map<Long, Boolean> via
@Query is unsafe for Spring Data JPA and can cause runtime mapping errors;
change the repository to return a List of a projection (create e.g.
ReservationConfirmedProjection with Long getRoomId(); boolean isConfirmed();) or
List<Object[]> if you prefer, add @Param("roomIds") on the method parameter, and
then convert the returned List<ReservationConfirmedProjection> to a Map<Long,
Boolean> in the service layer using Collectors.toMap(Projection::getRoomId,
Projection::isConfirmed).
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java (1)
58-62:@Param어노테이션을 추가하고 1:1 관계 가정을 확인하세요.이전 리뷰에서 권장했던
@Param("roomIds")어노테이션이 여전히 누락되어 있습니다. 명시적으로 파라미터 이름을 지정하면 컴파일 옵션과 무관하게 안정적으로 동작하며 유지보수성이 향상됩니다.또한 이 쿼리는 각
chatRoomId가 하나의Reservation과 1:1로 매핑된다고 가정합니다. PR 목표에서 언급된 것처럼 1:1 관계가 의도된 설계라면 문제없지만, 향후 한 채팅방에 여러 예약이 존재할 수 있다면 서비스 레이어에서 중복 처리 로직이 필요할 수 있습니다.@Query("SELECT r.chatRoom.chatRoomId AS roomId, " + "CASE WHEN r.reservationStatus = 'CONFIRMED' THEN true ELSE false END AS confirmed " + "FROM Reservation r " + "WHERE r.chatRoom.chatRoomId IN :roomIds") - List<ReservationConfirmedProjection> findReservationConfirmedByChatRoomIds(List<Long> roomIds); + List<ReservationConfirmedProjection> findReservationConfirmedByChatRoomIds( + @Param("roomIds") List<Long> roomIds);src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java (2)
60-78: 채팅방 참여자 검증이 부족해 임의 사용자가 상대 정보에 접근할 수 있습니다현재는
isOwner && user.getRole() != Role.OWNER만 막고 있어, 다른 푸드트럭의 사장(OWNER) 또는 일반 회원이roomId와isOwner값을 임의로 조작하면 자신이 속하지 않은 채팅방의 예약자/사장 이름과 푸드트럭 이름을 조회할 수 있습니다.roomId기준으로 실제 참여자인지(푸드트럭 사장 또는 예약자)까지 검증해야 합니다.아래처럼 방에 속한 사장/예약자 ID와
user.getUserId()를 비교하고, 불일치 시CHAT_ROOM_FORBIDDEN을 던지도록 변경하는 것을 제안합니다.ChatRoom chatRoom = chatRoomRepository.findById(roomId) .orElseThrow(() -> new EntityNotFoundException(CHAT_ROOM_NOT_FOUND)); Long reservationId = reservationRepository.findByChatRoom(chatRoom) .map(Reservation::getReservationId) .orElse(null); // 없으면 null - if(isOwner && user.getRole() != Role.OWNER) { - throw new BusinessException(USER_FORBIDDEN); - } - - // 푸드트럭 사장일 경우 예약자 이름 반환 - if(isOwner) return ChatRoomMetaDataResponse.of(chatRoom.getMember().getName(), null, reservationId); - - // 예약자일 경우 푸드트럭 사장 이름 및 푸드트럭 이름 반환 - FoodTruck foodTruck = chatRoom.getFoodTruck(); - return ChatRoomMetaDataResponse.of(foodTruck.getOwner().getName(), foodTruck.getFoodTruckInfo().getName(), reservationId); + Long ownerId = chatRoom.getFoodTruck().getOwner().getUserId(); + Long memberId = chatRoom.getMember().getUserId(); + + if (isOwner) { + // 사장 뷰 요청: 실제로 해당 푸드트럭 사장인지 검증 + if (!user.getRole().equals(Role.OWNER) || !user.getUserId().equals(ownerId)) { + throw new BusinessException(CHAT_ROOM_FORBIDDEN); + } + // 푸드트럭 사장일 경우 예약자 이름 반환 + return ChatRoomMetaDataResponse.of(chatRoom.getMember().getName(), null, reservationId); + } + + // 예약자 뷰 요청: 실제로 예약자인지 검증 + if (!user.getUserId().equals(memberId)) { + throw new BusinessException(CHAT_ROOM_FORBIDDEN); + } + + // 예약자일 경우 푸드트럭 사장 이름 및 푸드트럭 이름 반환 + FoodTruck foodTruck = chatRoom.getFoodTruck(); + return ChatRoomMetaDataResponse.of( + foodTruck.getOwner().getName(), + foodTruck.getFoodTruckInfo().getName(), + reservationId + );이렇게 하면 프런트에서
isOwner플래그를 잘못 보내더라도 실제 채팅방 참여자가 아닌 사용자는 정보를 얻지 못하게 됩니다.
80-132: isOwner null 시 NPE 및 RDB 조회 누락 시 NPE 가능성이 있습니다
isOwner의 Boolean → boolean 언박싱 NPE 가능성
getChatRooms의isOwner는Boolean인데, 그대로findChatRoomsForUser(..., isOwner, ...)/ChatRoomResponse.from(..., isOwner, ...)에 넘기면, 해당 인자 타입이boolean인 경우 자동 언박싱 과정에서NullPointerException이 날 수 있습니다. 기본값을 명시적으로 계산해 사용하는 편이 안전합니다.- public CursorPagingResponse<ChatRoomResponse> getChatRooms(User member, Boolean isOwner, Long cursor, Integer size) { + public CursorPagingResponse<ChatRoomResponse> getChatRooms(User member, Boolean isOwner, Long cursor, Integer size) { int pageSize = (size == null || size < 1) ? 20 : size; Long cursorSortKey = (cursor != null) ? cursor : Long.MAX_VALUE; int limit = pageSize + 1; - - // 1. MongoDB에서 메타데이터 + unreadCount 조회 - List<ChatRoomMetaDataProjection> metaList = - chatRoomMetaDataRepository.findChatRoomsForUser(member.getUserId(), isOwner, cursorSortKey, limit); + boolean ownerView = Boolean.TRUE.equals(isOwner); + + // 1. MongoDB에서 메타데이터 + unreadCount 조회 + List<ChatRoomMetaDataProjection> metaList = + chatRoomMetaDataRepository.findChatRoomsForUser(member.getUserId(), ownerView, cursorSortKey, limit); @@ - // 4. 메타데이터 + RDB 정보 조합해서 ChatRoomResponse 생성 - List<ChatRoomResponse> responses = metaList.stream() - .map(meta -> ChatRoomResponse.from(chatRoomMap.get(meta.getRoomId()), meta, isOwner, - reservationConfirmedMap.getOrDefault(meta.getRoomId(), false))) - .toList(); + // 4. 메타데이터 + RDB 정보 조합해서 ChatRoomResponse 생성 + List<ChatRoomResponse> responses = metaList.stream() + .map(meta -> ChatRoomResponse.from(chatRoomMap.get(meta.getRoomId()), meta, ownerView, + reservationConfirmedMap.getOrDefault(meta.getRoomId(), false))) + .toList();
- Mongo와 RDB 데이터 불일치 시 NPE 가능성
chatRoomMap.get(meta.getRoomId())가null을 반환해도 그대로ChatRoomResponse.from에 전달되고 있어, RDB에서 해당 roomId를 못 찾는 경우 NPE가 발생할 수 있습니다. 방어적으로 필터링 및 로그를 추가하는 편이 좋습니다.
@Slf4j를 쓰는 예시는 아래와 같습니다.-import lombok.RequiredArgsConstructor; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; @@ -@Service -@RequiredArgsConstructor -public class ChatRoomService { +@Service +@RequiredArgsConstructor +@Slf4j +public class ChatRoomService { @@ - // 4. 메타데이터 + RDB 정보 조합해서 ChatRoomResponse 생성 - List<ChatRoomResponse> responses = metaList.stream() - .map(meta -> ChatRoomResponse.from(chatRoomMap.get(meta.getRoomId()), meta, isOwner, - reservationConfirmedMap.getOrDefault(meta.getRoomId(), false))) - .toList(); + // 4. 메타데이터 + RDB 정보 조합해서 ChatRoomResponse 생성 + List<ChatRoomMetaDataProjection> filteredMetaList = metaList.stream() + .filter(meta -> { + if (!chatRoomMap.containsKey(meta.getRoomId())) { + log.warn("ChatRoom not found in RDB for roomId={} (skipping from response)", meta.getRoomId()); + return false; + } + return true; + }) + .toList(); + + List<ChatRoomResponse> responses = filteredMetaList.stream() + .map(meta -> ChatRoomResponse.from( + chatRoomMap.get(meta.getRoomId()), + meta, + ownerView, + reservationConfirmedMap.getOrDefault(meta.getRoomId(), false))) + .toList(); @@ - Long lastCursor = responses.isEmpty() ? null : metaList.get(metaList.size() - 1).getSortKey(); + Long lastCursor = responses.isEmpty() + ? null + : filteredMetaList.get(filteredMetaList.size() - 1).getSortKey();이렇게 하면 Mongo/RDB 데이터가 일부 불일치하더라도 서비스 전체가 NPE로 깨지지 않고, 로그를 통해 정합성 이슈를 추적할 수 있습니다.
🧹 Nitpick comments (3)
src/main/java/konkuk/chacall/domain/reservation/domain/repository/dto/ReservationConfirmedProjection.java (1)
3-8:getConfirmed()의 반환 타입을boolean원시형으로 변경하는 것을 고려하세요.JPQL 쿼리의
CASE WHEN ... THEN true ELSE false END표현식은 항상 non-null 값을 반환하므로,Boolean객체 타입 대신boolean원시형을 사용하는 것이 더 적합합니다. 이렇게 하면 불필요한 nullability를 제거하고 오토박싱 오버헤드를 방지할 수 있습니다.public interface ReservationConfirmedProjection { Long getRoomId(); - Boolean getConfirmed(); + boolean getConfirmed(); }src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java (1)
16-16: 사용하지 않는 import를 제거하세요.
Map타입이 더 이상 사용되지 않으므로 (이전Map<Long, Boolean>반환 타입에서List<ReservationConfirmedProjection>으로 리팩토링됨) 해당 import를 제거하는 것이 좋습니다.-import java.util.List; -import java.util.Map; +import java.util.List; import java.util.Optional;src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java (1)
41-55: 동일 member–푸드트럭 조합 동시 요청 시 중복 채팅방 생성 가능성
findByMemberAndFoodTruck(...).orElseGet(() -> save(...))패턴은 두 요청이 거의 동시에 들어올 경우 서로가 아직 커밋되지 않은 레코드를 못 보고 각각 새ChatRoom을 저장할 수 있습니다. Mongo 메타데이터도 동일하게 중복 생성될 여지가 있습니다.DB 레벨에서
(member_id, food_truck_id)유니크 인덱스를 두고,save시 제약 위반이 나면 재조회하는 패턴이나, JPA 락(@Lock(PESSIMISTIC_WRITE)등)을 이용한 조회로 중복 생성을 막는 방향을 검토해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java(1 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java(3 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/repository/dto/ReservationConfirmedProjection.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-22T08:13:37.857Z
Learnt from: buzz0331
Repo: CHA-CALL/ChaCall-Server PR: 30
File: src/main/java/konkuk/chacall/domain/reservation/application/ReservationService.java:52-68
Timestamp: 2025-09-22T08:13:37.857Z
Learning: ChaCall 프로젝트에서 OWNER 역할은 MEMBER 권한을 포함합니다. memberValidator.validateAndGetMember()는 MEMBER와 OWNER 모두를 허용하므로, OWNER가 예약 취소 등 MEMBER 기능을 사용할 수 있습니다.
Applied to files:
src/main/java/konkuk/chacall/domain/chat/application/room/ChatRoomService.java
🔇 Additional comments (1)
src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java (1)
56-56: 변경 사항이 적절합니다.
ChatRoom으로Reservation을 조회하는 메서드가 올바르게 구현되었습니다.Optional반환 타입은 1:1 관계에서 예약이 존재하지 않을 수 있는 경우를 잘 처리합니다.
#️⃣연관된 이슈
📝작업 내용
MongoDB에
ChatRoomMetaData를 추가했습니다. 추가한 이유는 다음과 같습니다.채팅방 목록 조회시에 가장 최근 메시지 순으로 정렬이 되어야 하는데 이때, 가장 최근 메시지 시각을 RDB에 저장해둘 경우 메시지 전송시마다 RDB가 업데이트가 되어야 하기 때문에 MongoDB를 사용하는 이점이 사라진다고 판단했습니다. 따라서, 채팅 입력시 ChatMessage 컬렉션에는 새로운 메시지가 insert되고 ChatRoomMetaData에는 최근 메시지가 발송된 시각, 최근 메시지 내용이 갱신되도록 하였습니다.
채팅방 생성 API
채팅방 상대 이름 조회
특이사항은 isOwner라는 쿼리 파라미터를 받는 것인데, 처음에는 User의 Role만으로 판단하면 되지 않을까 생각했지만 저희 서비스 특성상 사장님이 예약자가 될 수 있기 때문에 특정 사장님이 예약자 신분에서 어떤 푸드트럭을 예약하는 상황을 고려했을때, 로그인한 User의 Role만으로는 판단이 불가능하다고 판단하여 프론트로부터 현재 뷰가 사장님 뷰인지, 일반 유저 뷰인지를 통해 isOwner를 반환받을 수 있도록 하였습니다.
채팅방 목록 조회
메시지 내역 조회
현재 메시지의 전송시각 순으로 내림차순으로 정렬하여 페이징 처리를 하고 있는데, MongoDB의 Document 특성상 key가 문자열 기반이기 때문에 저희 커서 기반 페이지네이션 DTO와 연동이 되지 않을 것으로 판단되어 일단은 오프셋 기반 페이지네이션으로 처리하였습니다.
추후에 리팩토링이 필요해보입니다..
채팅방 내 메시지 읽음 처리
고민이 가장 많이 되었던 API인데 우선, 채팅방 진입시에 모든 메시지가 읽음 처리가 되어야 할 것 같아서 한번의 쿼리로 업데이트되게 해두었습니다. 다만, 당연히 성능이 우려되기 때문에 채팅방 내의 각 사용자의 최근 접속 시간을 저장하여 해당 접속 시간 이후에 도착한 메시지에 대해서만 읽음 처리를 하도록 구현하려고 했지만, 마땅한 방법이 떠오르지 않아서 우선은 기능 그대로 놔둔 상황입니다. 레퍼런스를 조금 찾아보니, Interceptor에서 Event를 발행하여 비동기적으로 처리하는 방법도 있는 것 같은데 조금더 찾아봐야 할 것 같습니다,,
채팅 전송 흐름
스크린샷 (선택)
Postman으로 확인할때 주의해야될게 메시지를 base64로 인코딩하여 넘겨야 했습니다.
참고한 레퍼런스입니다. 혹시 테스트하시려면 참고해주세요.
https://dev.to/danielsc/testing-stomp-websocket-with-postman-218a?utm_source=chatgpt.com
💬리뷰 요구사항(선택)
우선, 읽음처리를 어떻게 구현할지가 가장 고민이 되는 부분입니다. 여러 구현 방법이 있는 것 같은데 크게 2가지 정도로
"SUBSCRIBE와 UNSUBSCRIBE command를 각각 핸들링하여 현재 사용자의 채팅룸 접속 시간 갱신 -> 읽음 표시 API에서 접속 시간을 기준으로 읽음 처리"
가 정석적인 방법인 것 같습니다. 다만, 이를 Interceptor에서 처리해줘야 할 것 같은데 Interceptor에서 서비스 로직이 들어가면 좋지 않을 것 같아서 Spring Application 이벤트를 활용해서 의존성을 분리할까 생각중인데 이에 대해서 어떻게 생각하시나요??
또, 현재 ChatRoom과 Reservation의 연관관계가 맺어져있지 않았는데 저희 서비스에서는 Reservation은 ChatRoom과 1:1 대응 되어야 하는 것으로 파악했습니다. 따라서, Reservation에 ChatRoom을 OneToOne으로 걸고 예약 생성 API시에 프론트로부터 ChatRoomId를 받은다음 주입해줄 생각인데 이에 대한 상균님 의견도 궁금합니다.!!
구현 내용이 워낙 많았다보니, 제가 놓친 부분이 좀 있을 것 같은데 서비스 로직 위주로 한번 살펴봐주시면 감사하겠습니다 ㅠ
Summary by CodeRabbit
New Features
Chores
Other
✏️ Tip: You can customize this high-level summary in your review settings.