refactor: [ALT-270] 채팅 개선 그룹 단톡·읽음처리·presence·이미지·RabbitMQ relay#95
Open
hodoon wants to merge 35 commits into
Open
refactor: [ALT-270] 채팅 개선 그룹 단톡·읽음처리·presence·이미지·RabbitMQ relay#95hodoon wants to merge 35 commits into
hodoon wants to merge 35 commits into
Conversation
Task 2A/2B에서 변경된 ChatRoom/ChatMessage/ChatRoomMember 엔티티에 맞춰 실 Postgres 스키마를 정합화한다. - chat_rooms: type, workspace_id 컬럼 추가 - chat_messages: type 컬럼 추가 - chat_rooms: participant1/2_id, participant1/2_scope NOT NULL 해제 (그룹 채팅은 멤버 테이블로 관리하므로 참가자 컬럼을 쓰지 않음) - chat_room_members 테이블 신설 및 기존 1:1 방 참가자 백필
- ChatRoomQueryRepository에 findGroupRoomByWorkspaceId 추가 (QueryDSL) - SyncWorkspaceChatMembershipUseCase 인바운드 포트 및 구현 추가 - join 멱등성(활성 멤버 중복 생성 방지, 퇴장 멤버 rejoin) 단위 테스트 작성
업장 등록 신청이 승인되어 Workspace가 생성되는 시점에 SyncWorkspaceChatMembershipUseCase를 호출해 그룹 채팅방을 생성하고 업장 매니저를 해당 방에 입장시킨다. reject 경로에는 영향 없음.
- AddWorkerToWorkspace: 워커 추가 저장 직후 syncWorkspaceChatMembership.join 호출 (APP scope) - WorkerResignationServiceImpl: 퇴사 처리(worker.resign()) 직후 syncWorkspaceChatMembership.leave 호출 (APP scope) - ManagerResignWorkspaceWorker, UserResignWorkspaceWorker 모두 이 서비스에 위임하므로 한 곳의 훅으로 양쪽 퇴사 플로우 커버
- AbstractSendChatMessageUseCase에 ChatRoomMemberQueryRepository 주입, findById로 방을 먼저 조회한 뒤 DIRECT는 기존 findByIdAndParticipant, GROUP은 existsActive로 참여자 검증하는 이중 경로 구조로 리팩터링 - NOTICE 타입 메시지는 MANAGER만 전송 가능하도록 검증 (CHAT_NOTICE_FORBIDDEN) - SendChatMessageRequestDto에 type 필드 추가 (null이면 NORMAL로 취급) - FCM 알림은 DIRECT 방에서만 발송 (그룹 팬아웃은 Phase 5로 이연) - SendChatMessage/ManagerSendChatMessage 생성자에 새 의존성 주입 배선
멤버가 채팅방을 특정 메시지까지 읽음 처리할 수 있도록 MarkChatRoomReadUseCase를 추가한다.
lastReadMessageId는 과거 값으로 되돌아가지 않도록 멱등하게 갱신되며,
이후 Task 4.2에서 이 값을 기준으로 메시지별 안읽음 수를 계산한다.
- domain/chat/port/inbound/MarkChatRoomReadUseCase 추가
- application/chat/usecase/MarkChatRoomRead 구현 (memberId+scope 기반, 단일 빈으로 앱/매니저 공용)
- POST /app/chat/rooms/{chatRoomId}/read, POST /manager/chat/rooms/{chatRoomId}/read 엔드포인트 추가
SyncWorkspaceChatMembership.join 호출에 ManagerUser 엔티티 id를 넘기고 있어, 채팅 도메인이 참여자 식별에 사용하는 User(계정) id와 불일치했다. 이로 인해 매니저가 자신의 업장 단톡방에 공지를 보내거나(existsActive 조회 실패) 읽음 처리를 해도(MarkChatRoomRead) 멤버십 행을 찾지 못하는 문제가 있었다. AbstractChatUseCase.getParticipantId(ManagerUser) 컨벤션에 맞춰 managerUser.getUser().getId()를 사용하도록 수정.
메시지 목록 조회 시 각 메시지의 unreadCount(활성 멤버 중 아직 읽지 않고 발신자 본인이 아닌 인원 수)를 계산해 응답에 포함한다. 방 활성 멤버 목록을 쿼리 1회로 로드해 메모리에서 메시지별로 계산하여 N+1을 방지한다. 실시간 브로드캐스트 페이로드는 범위 밖.
STOMP connect/disconnect 리스너(5.2)와 push fallback(5.3)에서 쓸
온라인 상태 포트/어댑터 추가. EmailVerificationSessionStoreRepositoryImpl
컨벤션을 따라 domain/chat/port/outbound에 포트, adapter/outbound/chat/redis에
StringRedisTemplate 기반 구현체를 둔다. TTL 60초, 키는 chat:presence:{SCOPE}:{memberId}.
CONNECT 시 Principal에서 (scope, memberId)를 추출해 ChatPresenceStore.markOnline, DISCONNECT 시 markOffline을 호출하는 WebSocketEventListener 추가. Principal → LoginUserDto 추출은 UserChatWebSocketController 패턴을 재사용.
- DIRECT: 상대방이 온라인이면 FCM 알림 생략 (WebSocket으로 이미 수신) - GROUP: 발신자를 제외한 활성 멤버 중 오프라인 멤버에게만 FCM 팬아웃 발송 (기존엔 GROUP 전체 스킵) - ChatPresenceStore 포트를 AbstractSendChatMessageUseCase에 주입, SendChatMessage/ManagerSendChatMessage 생성자에 배선
chat.broker.relay.enabled 프로퍼티로 SimpleBroker/RabbitMQ relay를 전환한다. 기본값은 false로 두어 기존 테스트/배포는 SimpleBroker 경로를 그대로 사용하고, RabbitMQ(STOMP 플러그인) 배포 후 CHAT_BROKER_RELAY_ENABLED=true로 전환 가능하도록 한다. 오프라인 gradle 캐시 제약으로 relay 런타임 연결에 필요한 reactor-netty-core만 추가한다 (netty-all은 캐시에 없어 오프라인 빌드가 깨지므로 제외).
RabbitMQ(STOMP) 서비스는 워크스페이스 루트 local-infra/docker-compose.yml에 추가함. relay 코드(build.gradle, WebSocketConfig, application.yml)는 유지.
이미지만 있는 메시지를 허용하기 위해 content NOT NULL 제약을 완화. DTO/검증/첨부 연동은 후속 태스크(6.2/6.3)에서 처리.
이미지 전용 메시지를 지원하기 위해 content의 @notblank 제약을 제거하고 fileIds 필드를 추가한다. content/fileIds 중 하나는 필수라는 검증은 usecase 단에서 처리한다(Task 6.3). 첨부 관련 ErrorCode(B027, B028) 추가.
- AbstractSendChatMessageUseCase에 AttachFilesUseCase/FileQueryRepository/FileUrlService 주입 - 내용·이미지 모두 없으면 CHAT_EMPTY_MESSAGE, 첨부 10개 초과시 CHAT_TOO_MANY_ATTACHMENTS 검증 추가 - 메시지 저장 후 fileIds를 CHAT_MESSAGE 타겟으로 연결하고, 브로드캐스트 payload(ChatMessageResponse)에 FileResponseDto 목록(attachments)을 포함해 실시간으로 첨부 URL 전달 - ChatMessageResponse는 QueryDSL Projections.constructor 호환을 위해 명시적 6-arg 생성자를 유지하고 attachments 필드는 @Setter로 추가 (프로젝션 안전) - SendChatMessage/ManagerSendChatMessage 생성자에 3개 의존성 추가 및 관련 테스트 보강
메시지 목록 조회 시 페이지 내 메시지 id를 모아 첨부파일을 findAllByTargetTypeAndTargetIdIn으로 한 번에 조회하고 targetId 기준으로 그룹핑하여 각 메시지에 매핑한다 (N+1 방지).
이미지-only 채팅 메시지의 FCM 푸시 body가 "senderName: "으로 어색하게 비어있던 문제 수정. content가 없으면 "senderName: 사진을 보냈습니다"로 표시하고, 텍스트 메시지는 기존 미리보기 body를 유지한다. DIRECT/GROUP FCM 발송 두 경로에서 중복되던 body 조합 로직을 buildNotificationBody() 헬퍼로 통합.
AbstractCreateOrGetChatRoomUseCase가 기존 방이 없어 새 DIRECT 방을 생성할 때 chat_room_members 행을 만들지 않아, MarkChatRoomRead가 404를 반환하고 unreadCount가 항상 0으로 계산되는 문제를 수정. - ChatRoomMemberRepository를 주입받아 새 방 저장 직후 참여자 2명의 ChatRoomMember를 saveAll로 생성 (기존 방 재사용 시에는 생성하지 않음, V4에서 이미 백필됨) - CreateOrGetChatRoom, ManagerCreateOrGetChatRoom 생성자에 의존성 추가
SyncWorkspaceChatMembership의 join/leave가 단톡방이 없으면 NOT_FOUND 예외를 던져, 업장 @transactional 안에서 호출되는 AddWorkerToWorkspace/WorkerResignationServiceImpl 트랜잭션이 롤백되는 문제를 수정. GROUP 방은 UpdateWorkspaceRequestStatus에서만 생성되므로, 그 전에 존재하던 레거시 업장은 방이 없었음. - join: resolveGroupRoom(예외 발생) 대신 createGroupRoom(find-or-create) 사용 — 레거시 업장 워커가 join 할 때 단톡방을 지연 생성 - leave: 단톡방이 없으면 정리할 멤버십도 없으므로 조용히 no-op - 더 이상 쓰이지 않는 resolveGroupRoom 제거
GROUP 채팅방은 participant1/2 컬럼이 null이라 findByIdAndParticipant가 항상 실패해 정상 멤버도 NOT_FOUND를 받는 버그를 수정. AbstractSendChatMessageUseCase와 동일하게 findById로 방을 조회한 뒤 GROUP이면 chatRoomMemberQueryRepository.existsActive로, DIRECT면 기존 findByIdAndParticipant로 인증하도록 분기.
클라이언트가 업장의 그룹 채팅방 id를 얻어 /sub/chat.{roomId} 구독 및
히스토리 조회를 시작할 수 있도록 GET /app/chat/workspace/{workspaceId}/room,
GET /manager/chat/workspace/{workspaceId}/room 엔드포인트를 추가한다.
요청자가 해당 그룹 채팅방의 활성 멤버일 때만 room id를 반환하며,
아직 방이 생성되지 않은 레거시 업장은 NOT_FOUND로 응답한다(백필은 별도 작업).
- ChatMembershipJoinedEvent/LeftEvent + ChatMembershipSyncEventListener 추가 - UpdateWorkspaceRequestStatus, AddWorkerToWorkspace, WorkerResignationServiceImpl 에서 SyncWorkspaceChatMembershipUseCase 인라인 호출을 이벤트 발행으로 교체 - 리스너는 @TransactionalEventListener(AFTER_COMMIT) + try/catch 로 best-effort 동기화 수행 (채팅 동기화 실패가 업장 승인/입사/퇴사 같은 코어 트랜잭션을 롤백시키지 않도록 완전 분리) - join()이 createGroupRoom을 self-heal 하므로(C1) 별도 createGroupRoom 호출 제거
…tty) reactor-netty-core만으론 Spring StompBrokerRelay가 'No compatible version of Reactor Netty'로 기동 실패. reactor-netty 애그리게이트로 정정해 relay TCP 연결 정상화. Docker 실환경 검증(CHAT_BROKER_RELAY_ENABLED=true)으로 확인.
배포 이전 존재하던 업장에 GROUP 방과 멤버(오너 매니저 + 활성 워커)를 생성. 신규 업장은 활성화 이벤트로 자동 생성되므로 GROUP 방이 없는 기존 업장만 대상. Docker 실환경(8업장/8워커/4매니저)에서 rollback 트랜잭션으로 검증: GROUP방 8, 매니저멤버 8, 활성워커멤버 6, 재실행 멱등(유니크 위반 없음).
📝 WalkthroughWalkthrough그룹 채팅방과 멤버십 모델, 첨부·읽음 처리, presence 기반 알림, 워크스페이스 이벤트 동기화, 사용자·매니저 API 및 STOMP broker relay 설정이 추가되었습니다. Changes채팅 모델과 저장 구조
멤버십 동기화와 presence
메시지 전송과 조회
채팅 API와 브로커 설정
기능 검증
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
hodoon
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
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/dreamteam/alter/application/chat/usecase/AbstractSendChatMessageUseCase.java (1)
53-141: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
@Transactional내 WebSocket/FCM 전송 부작용 — DB 커넥션 점유 및 데이터 불일치 위험.
execute메서드가@Transactional로 실행되므로 WebSocket 전송(line 131)과 FCM 전송(lines 137-141)이 트랜잭션 내에서 수행됩니다. 이는 두 가지 문제를 발생시킵니다:
- DB 커넥션 장시간 점유: FCM 전송은 네트워크 호출이며, 특히 그룹방의 경우 N명 멤버에 대해 루프 내 전송(line 208)이 수행됩니다. 트랜잭션이 DB 커넥션을 잡고 있는 동안 이 작업들이 실행되어 커넥션 풀 고갈 위험이 있습니다.
- 커밋 실패 시 데이터 불일치: 트랜잭션 커밋 실패 시 이미 WebSocket/FCM으로 전송된 메시지가 DB에 존재하지 않게 됩니다.
PR에서 이미 멤버십 동기화에
AFTER_COMMIT이벤트 패턴을 사용하고 있으므로, 메시지 전송 부작용에도 동일한 패턴 적용을 권장합니다.♻️ 제안하는 리팩토링 방향
// execute 메서드 내 DB 작업 완료 후: chatRoom.updateUpdatedAt(); -// 6. WebSocket으로 실시간 전송 -try { - ChatMessageResponse messageResponse = new ChatMessageResponse(...); - messageResponse.setAttachments(attachments); - messagingTemplate.convertAndSend("/sub/chat." + chatRoom.getId(), messageResponse); -} catch (Exception e) { - log.error("WebSocket 메시지 전송 실패..."); -} - -// 7. FCM 알림 전송 -if (chatRoom.getType() == ChatRoomType.DIRECT) { - sendDirectFcmNotification(...); -} else { - sendGroupFcmNotification(...); -} +// AFTER_COMMIT 이벤트 발행 +eventPublisher.publishEvent(new ChatMessageSentEvent( + savedMessage, chatRoom, senderId, senderScope, attachments));// 별도 이벤트 리스너 `@Component` `@RequiredArgsConstructor` `@Slf4j` public class ChatMessageSentEventListener { `@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT) public void onChatMessageSent(ChatMessageSentEvent event) { // WebSocket 전송 + FCM 전송 로직을 여기로 이동 } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/application/chat/usecase/AbstractSendChatMessageUseCase.java` around lines 53 - 141, Move the WebSocket and FCM side effects out of AbstractSendChatMessageUseCase.execute and publish a ChatMessageSentEvent containing the saved message, chat room, sender details, content, and attachments within the transaction. Add a separate ChatMessageSentEventListener with an `@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT) that performs the existing messagingTemplate.convertAndSend and sendDirectFcmNotification/sendGroupFcmNotification logic, preserving error handling and required data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/chat/controller/UserChatController.java`:
- Around line 92-101: The markChatRoomRead request body is not triggering DTO
validation. Add `@Valid` to the MarkChatRoomReadRequestDto parameter in
UserChatController.markChatRoomRead, preserving the existing `@RequestBody`
annotation and imports so lastReadMessageId is validated before the use case
executes.
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/manager/chat/controller/ManagerChatController.java`:
- Around line 91-100: ManagerChatController의 markChatRoomRead 메서드에서 `@RequestBody`
MarkChatRoomReadRequestDto 파라미터에 `@Valid를` 추가하여 DTO의 `@NotNull` 등 Bean Validation이
실행되도록 수정하세요.
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/chat/persistence/ChatRoomQueryRepositoryImpl.java`:
- Around line 157-170: findGroupRoomByWorkspaceId에서 중복 GROUP 방으로 fetchOne()이 예외를
발생시키지 않도록 fetchFirst()로 변경하세요. 동시에 생성되는 중복을 방지하려면 ChatRoom 생성 경로의 선조회 후 저장 로직에
락/업서트를 적용하고, 가능하면 type/workspace_id 조합에 DB unique constraint를 추가해 단일성을 보장하세요.
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/chat/persistence/readonly/ChatMessageResponse.java`:
- Around line 12-38: Remove the class-level `@Setter` from ChatMessageResponse and
apply `@Setter`(AccessLevel.PRIVATE) only to the attachments field, adding the
required AccessLevel import so other response fields do not expose setters while
attachments remains configurable for post-processing.
In
`@src/main/java/com/dreamteam/alter/application/chat/event/ChatMembershipSyncEventListener.java`:
- Around line 17-24: ChatMembershipSyncEventListener의 onJoined 및 관련 동기화 이벤트 처리에서
실패를 로그만 남기지 말고 운영 모니터링 알람을 연동하거나 재시도 가능한 큐/DLQ로 전달하도록 보완하세요. 기존 예외 로깅은 유지하되, 실패
이벤트와 workspaceId, memberId, scope를 알람 또는 재처리 메시지에 포함하고 중복 처리에 안전한 방식으로 구현하세요.
In `@src/main/java/com/dreamteam/alter/common/config/WebSocketConfig.java`:
- Around line 19-44: Replace the `@Value` fields in WebSocketConfig with an
injected `@ConfigurationProperties-based` ChatBrokerRelayProperties object,
preserving the existing defaults and using its accessors in
configureMessageBroker. Register the properties class for Spring configuration
binding, and when relayEnabled is true, explicitly configure system heartbeat
send and receive intervals on the STOMP relay.
In `@src/main/java/com/dreamteam/alter/common/config/WebSocketEventListener.java`:
- Around line 28-32: 다중 WebSocket 세션이 있는 사용자의 연결 종료 시 presence를 즉시 오프라인 처리하지 않도록
수정하세요. WebSocketEventListener의 onDisconnect와 ChatPresenceStore를 변경해 사용자별 세션 ID
참조 카운트 또는 세션 집합을 관리하고, 해당 사용자의 모든 세션이 종료되어 집합이 비었을 때만 markOffline을 호출하세요.
In `@src/main/java/com/dreamteam/alter/domain/chat/entity/ChatRoom.java`:
- Around line 27-46: Remove the JPA annotations from the domain entity ChatRoom,
including `@Column` and `@Enumerated` on its fields, and eliminate any related JPA
imports. Move persistence mapping to the infrastructure/persistence entity or
mapping layer, preserving column names, enum storage, lengths, and nullability
while keeping ChatRoom free of infrastructure dependencies.
- Around line 71-77: ChatRoom.createGroup must enforce the GROUP workspace
invariant by rejecting a null workspaceId before building the entity. Add an
appropriate null validation with a clear exception message, while preserving the
existing GROUP type and member-handling behavior.
In `@src/main/java/com/dreamteam/alter/domain/chat/entity/ChatRoomMember.java`:
- Around line 4-18: ChatRoomMember currently contains JPA and Spring auditing
dependencies, violating the domain layer boundary. Keep ChatRoomMember as a pure
domain model, create an adapter-layer ChatRoomMemberJpaEntity for persistence
annotations and auditing, and implement conversions in
ChatRoomMemberRepositoryImpl. Update ChatRoomMemberJpaRepository and
ChatRoomMemberQueryRepositoryImpl to use the persistence entity and map results
to and from ChatRoomMember.
In
`@src/test/java/com/dreamteam/alter/adapter/outbound/chat/persistence/ChatRoomMemberRepositoryImplTest.java`:
- Around line 16-22: Rename the test class and its file from
ChatRoomMemberRepositoryImplTest to ChatRoomMemberRepositoryImplTests, updating
the class declaration and any references accordingly to follow the required
plural “Tests” naming convention.
In
`@src/test/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImplTest.java`:
- Line 22: Rename the test class ChatPresenceStoreRepositoryImplTest to
ChatPresenceStoreRepositoryImplTests and update the corresponding filename and
any references to follow the required plural “Tests” naming convention.
In
`@src/test/java/com/dreamteam/alter/application/chat/usecase/GetChatMessagesTest.java`:
- Around line 46-240: Rename the test class from GetChatMessagesTest to
GetChatMessagesTests and update the filename accordingly. Replace every
verify(...) assertion in the test methods with the equivalent BDD Mockito
then(mock).should(...) form. Group related methods under `@Nested` classes with
`@DisplayName`, separating unread-count calculation, attachment mapping, and GROUP
access-control scenarios; retain the explicit ObjectMapper construction.
In
`@src/test/java/com/dreamteam/alter/application/chat/usecase/GetWorkspaceGroupChatRoomTest.java`:
- Around line 1-78: Rename the test file from GetWorkspaceGroupChatRoomTest.java
to GetWorkspaceGroupChatRoomTests.java and update the class declaration from
GetWorkspaceGroupChatRoomTest to GetWorkspaceGroupChatRoomTests, preserving all
existing tests and behavior.
In
`@src/test/java/com/dreamteam/alter/application/chat/usecase/ManagerSendChatMessageTest.java`:
- Around line 49-51: Rename the test class ManagerSendChatMessageTest to
ManagerSendChatMessageTests, including the file and class references, to follow
the plural naming convention. In the same class, group the six test methods into
`@Nested` classes with descriptive `@DisplayName` annotations for NOTICE/NORMAL
messages, exception cases, and attachment cases.
In
`@src/test/java/com/dreamteam/alter/application/chat/usecase/MarkChatRoomReadTest.java`:
- Around line 23-25: Rename the test class and its file from
MarkChatRoomReadTest to MarkChatRoomReadTests, updating the class declaration
and any references to follow the required plural “Tests” naming convention.
In
`@src/test/java/com/dreamteam/alter/application/chat/usecase/SendChatMessageTest.java`:
- Line 50: Rename the test class `SendChatMessageTest` and its file to
`SendChatMessageTests`, updating any references accordingly; apply the same
singular `Test` to plural `Tests` convention to other affected test classes such
as `ChatPresenceStoreRepositoryImplTest` and `CreateOrGetChatRoomTest`.
In
`@src/test/java/com/dreamteam/alter/application/chat/usecase/SyncWorkspaceChatMembershipTest.java`:
- Line 30: Rename the test class and its file from
SyncWorkspaceChatMembershipTest to SyncWorkspaceChatMembershipTests, updating
the class declaration and any references to follow the required plural Tests
suffix.
- Around line 47-132: 리뷰 지적대로 leave와 createGroupRoom의 핵심 시나리오 테스트를 보강하세요.
LeaveTests에 활성 ChatRoomMember가 조회될 때 leave()로 비활성화하고
chatRoomMemberRepository.save(...)가 호출되는 테스트를 추가하고, createGroupRoom 테스트에서는 새 그룹
채팅방이 올바른 속성으로 생성·저장되는지와 반환값을 검증하세요. 기존 테스트의 mock 저장소 설정과 sut 메서드 호출 패턴을 참고해 성공
경로를 커버하십시오.
In
`@src/test/java/com/dreamteam/alter/application/workspace/usecase/AddWorkerToWorkspaceTest.java`:
- Around line 31-33: Rename the test class and file from
AddWorkerToWorkspaceTest to AddWorkerToWorkspaceTests, updating the class
declaration and any references to follow the required [ClassName]Tests naming
convention.
- Around line 52-71: Update the test setup in execute_신규추가_단톡join이벤트발행 to use
BDD Mockito consistently: replace each when(...).thenReturn(...) call for
workspace, user, and workspaceWorkerQueryRepository with
given(...).willReturn(...), while retaining the existing then(...).should()
verification.
- Around line 73-89: Update the setup in execute_이미근무중_단톡join이벤트미발행 and the
other affected test to use BDD Mockito consistently: replace
when(...).thenReturn(...) with given(...).willReturn(...), while retaining the
existing then(...).should(...) verification.
In
`@src/test/java/com/dreamteam/alter/common/config/WebSocketEventListenerTest.java`:
- Around line 49-115: Group the WebSocket event tests into `@Nested` classes for
readability: place the onConnected_* tests under a nested class named
OnConnected and the onDisconnect_* tests under OnDisconnect, preserving all
existing assertions and test behavior.
- Line 31: Rename the test class WebSocketEventListenerTest to
WebSocketEventListenerTests and update the corresponding Java filename and any
references to follow the required [ClassName]Tests naming convention.
In `@src/test/java/com/dreamteam/alter/domain/chat/entity/ChatRoomTest.java`:
- Around line 8-17: Rename the test class and file from ChatRoomTest to
ChatRoomTests to follow the plural naming convention. Update the test method to
use explicit Given-When-Then comments and add an appropriate `@DisplayName`
annotation; use `@Nested` with `@DisplayName` if grouping related tests in
ChatRoomTests.
---
Outside diff comments:
In
`@src/main/java/com/dreamteam/alter/application/chat/usecase/AbstractSendChatMessageUseCase.java`:
- Around line 53-141: Move the WebSocket and FCM side effects out of
AbstractSendChatMessageUseCase.execute and publish a ChatMessageSentEvent
containing the saved message, chat room, sender details, content, and
attachments within the transaction. Add a separate ChatMessageSentEventListener
with an `@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT) that
performs the existing messagingTemplate.convertAndSend and
sendDirectFcmNotification/sendGroupFcmNotification logic, preserving error
handling and required data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1ccd302b-dfed-4358-9f73-1fd9b9ead945
📒 Files selected for processing (69)
.gitignorebuild.gradlesrc/main/java/com/dreamteam/alter/adapter/inbound/general/chat/controller/UserChatController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/chat/controller/UserChatControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/chat/dto/ChatMessageResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/chat/dto/MarkChatRoomReadRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/chat/dto/SendChatMessageRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/chat/controller/ManagerChatController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/chat/controller/ManagerChatControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/outbound/chat/persistence/ChatRoomMemberJpaRepository.javasrc/main/java/com/dreamteam/alter/adapter/outbound/chat/persistence/ChatRoomMemberQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/chat/persistence/ChatRoomMemberRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/chat/persistence/ChatRoomQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/chat/persistence/readonly/ChatMessageResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/chat/event/ChatMembershipJoinedEvent.javasrc/main/java/com/dreamteam/alter/application/chat/event/ChatMembershipLeftEvent.javasrc/main/java/com/dreamteam/alter/application/chat/event/ChatMembershipSyncEventListener.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/AbstractCreateOrGetChatRoomUseCase.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/AbstractGetChatMessagesUseCase.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/AbstractSendChatMessageUseCase.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/CreateOrGetChatRoom.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/GetChatMessages.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/GetWorkspaceGroupChatRoom.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/ManagerCreateOrGetChatRoom.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/ManagerGetChatMessages.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/ManagerSendChatMessage.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/MarkChatRoomRead.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/SendChatMessage.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/SyncWorkspaceChatMembership.javasrc/main/java/com/dreamteam/alter/application/workspace/usecase/AddWorkerToWorkspace.javasrc/main/java/com/dreamteam/alter/application/workspace/usecase/UpdateWorkspaceRequestStatus.javasrc/main/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImpl.javasrc/main/java/com/dreamteam/alter/common/config/WebSocketConfig.javasrc/main/java/com/dreamteam/alter/common/config/WebSocketEventListener.javasrc/main/java/com/dreamteam/alter/common/exception/ErrorCode.javasrc/main/java/com/dreamteam/alter/common/notification/NotificationMessageConstants.javasrc/main/java/com/dreamteam/alter/domain/chat/entity/ChatMessage.javasrc/main/java/com/dreamteam/alter/domain/chat/entity/ChatRoom.javasrc/main/java/com/dreamteam/alter/domain/chat/entity/ChatRoomMember.javasrc/main/java/com/dreamteam/alter/domain/chat/port/inbound/GetWorkspaceGroupChatRoomUseCase.javasrc/main/java/com/dreamteam/alter/domain/chat/port/inbound/MarkChatRoomReadUseCase.javasrc/main/java/com/dreamteam/alter/domain/chat/port/inbound/SyncWorkspaceChatMembershipUseCase.javasrc/main/java/com/dreamteam/alter/domain/chat/port/outbound/ChatPresenceStore.javasrc/main/java/com/dreamteam/alter/domain/chat/port/outbound/ChatRoomMemberQueryRepository.javasrc/main/java/com/dreamteam/alter/domain/chat/port/outbound/ChatRoomMemberRepository.javasrc/main/java/com/dreamteam/alter/domain/chat/port/outbound/ChatRoomQueryRepository.javasrc/main/java/com/dreamteam/alter/domain/chat/type/ChatMessageType.javasrc/main/java/com/dreamteam/alter/domain/chat/type/ChatRoomType.javasrc/main/resources/application.ymlsrc/main/resources/db/migration/V4__unify_chat_schema.sqlsrc/main/resources/db/migration/V5__chat_message_content_nullable.sqlsrc/main/resources/db/migration/V6__backfill_workspace_group_chat.sqlsrc/test/java/com/dreamteam/alter/adapter/outbound/chat/persistence/ChatRoomMemberRepositoryImplTest.javasrc/test/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImplTest.javasrc/test/java/com/dreamteam/alter/application/chat/event/ChatMembershipSyncEventListenerTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/CreateOrGetChatRoomTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/GetChatMessagesTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/GetWorkspaceGroupChatRoomTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/ManagerSendChatMessageTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/MarkChatRoomReadTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/SendChatMessageTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/SyncWorkspaceChatMembershipTest.javasrc/test/java/com/dreamteam/alter/application/workspace/usecase/AddWorkerToWorkspaceTest.javasrc/test/java/com/dreamteam/alter/application/workspace/usecase/UpdateWorkspaceRequestStatusTests.javasrc/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTest.javasrc/test/java/com/dreamteam/alter/common/config/WebSocketEventListenerTest.javasrc/test/java/com/dreamteam/alter/domain/chat/entity/ChatRoomTest.javasrc/test/resources/application.yml
Comment on lines
12
to
+38
| @Getter | ||
| @Setter | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public class ChatMessageResponse { | ||
| private Long id; | ||
| private Long chatRoomId; | ||
| private Long senderId; | ||
| private TokenScope senderScope; | ||
| private String content; | ||
| private LocalDateTime createdAt; | ||
| private List<FileResponseDto> attachments; | ||
|
|
||
| public ChatMessageResponse( | ||
| Long id, | ||
| Long chatRoomId, | ||
| Long senderId, | ||
| TokenScope senderScope, | ||
| String content, | ||
| LocalDateTime createdAt | ||
| ) { | ||
| this.id = id; | ||
| this.chatRoomId = chatRoomId; | ||
| this.senderId = senderId; | ||
| this.senderScope = senderScope; | ||
| this.content = content; | ||
| this.createdAt = createdAt; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
@Setter 적용 범위 검토
클래스 레벨 @Setter로 인해 id, senderId 등 모든 필드에 setter가 노출됩니다. 실제로는 attachments만 후처리 세팅이 필요하므로, @Setter(AccessLevel.PRIVATE)를 attachments 필드에만 적용하는 것이 캡슐화에 유리합니다. 다만 현재 동작에는 문제가 없으므로 우선순위는 낮습니다.
♻️ 제안: attachments 필드에만 Setter 적용
`@Getter`
-@Setter
`@NoArgsConstructor`
public class ChatMessageResponse {
private Long id;
private Long chatRoomId;
private Long senderId;
private TokenScope senderScope;
private String content;
private LocalDateTime createdAt;
+
+ `@Setter`
private List<FileResponseDto> attachments;📝 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.
Suggested change
| @Getter | |
| @Setter | |
| @NoArgsConstructor | |
| @AllArgsConstructor | |
| public class ChatMessageResponse { | |
| private Long id; | |
| private Long chatRoomId; | |
| private Long senderId; | |
| private TokenScope senderScope; | |
| private String content; | |
| private LocalDateTime createdAt; | |
| private List<FileResponseDto> attachments; | |
| public ChatMessageResponse( | |
| Long id, | |
| Long chatRoomId, | |
| Long senderId, | |
| TokenScope senderScope, | |
| String content, | |
| LocalDateTime createdAt | |
| ) { | |
| this.id = id; | |
| this.chatRoomId = chatRoomId; | |
| this.senderId = senderId; | |
| this.senderScope = senderScope; | |
| this.content = content; | |
| this.createdAt = createdAt; | |
| } | |
| `@Getter` | |
| `@NoArgsConstructor` | |
| public class ChatMessageResponse { | |
| private Long id; | |
| private Long chatRoomId; | |
| private Long senderId; | |
| private TokenScope senderScope; | |
| private String content; | |
| private LocalDateTime createdAt; | |
| `@Setter` | |
| private List<FileResponseDto> attachments; | |
| public ChatMessageResponse( | |
| Long id, | |
| Long chatRoomId, | |
| Long senderId, | |
| TokenScope senderScope, | |
| String content, | |
| LocalDateTime createdAt | |
| ) { | |
| this.id = id; | |
| this.chatRoomId = chatRoomId; | |
| this.senderId = senderId; | |
| this.senderScope = senderScope; | |
| this.content = content; | |
| this.createdAt = createdAt; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/chat/persistence/readonly/ChatMessageResponse.java`
around lines 12 - 38, Remove the class-level `@Setter` from ChatMessageResponse
and apply `@Setter`(AccessLevel.PRIVATE) only to the attachments field, adding the
required AccessLevel import so other response fields do not expose setters while
attachments remains configurable for post-processing.
Comment on lines
+17
to
+24
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | ||
| public void onJoined(ChatMembershipJoinedEvent event) { | ||
| try { | ||
| syncWorkspaceChatMembership.join(event.getWorkspaceId(), event.getMemberId(), event.getScope()); | ||
| } catch (Exception e) { | ||
| log.error("업장 채팅 멤버십 join 동기화 실패. workspaceId={}, memberId={}, scope={}", | ||
| event.getWorkspaceId(), event.getMemberId(), event.getScope(), e); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
동기화 실패에 대한 모니터링/알람 추가를 권장합니다.
AFTER_COMMIT 이벤트에서 예외를 catch하고 로그만 남기는 것은 트랜잭션 영향도 없고 설계 의도에 부합합니다. 다만, 동기화 실패가 로그에만 남고 재시도나 알람 메커니즘이 없어, 운영 환경에서 멤버십 불일치가 장기간 방치될 수 있습니다.
최소한 이 에러 로그에 대한 모니터링 알람을 설정하거나, 실패 시 재시도 큐(DLQ) 적용을 고려해 보시기 바랍니다.
Also applies to: 27-34
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/dreamteam/alter/application/chat/event/ChatMembershipSyncEventListener.java`
around lines 17 - 24, ChatMembershipSyncEventListener의 onJoined 및 관련 동기화 이벤트
처리에서 실패를 로그만 남기지 말고 운영 모니터링 알람을 연동하거나 재시도 가능한 큐/DLQ로 전달하도록 보완하세요. 기존 예외 로깅은
유지하되, 실패 이벤트와 workspaceId, memberId, scope를 알람 또는 재처리 메시지에 포함하고 중복 처리에 안전한 방식으로
구현하세요.
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| @DisplayName("WebSocketEventListener 테스트") | ||
| class WebSocketEventListenerTest { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
테스트 클래스 명명 규칙 위반: Tests (복수) 접미사를 사용해야 합니다.
As per coding guidelines, 테스트 클래스는 [ClassName]Tests.java 명명 규칙을 따라야 합니다. WebSocketEventListenerTest → WebSocketEventListenerTests로 변경해 주세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/dreamteam/alter/common/config/WebSocketEventListenerTest.java`
at line 31, Rename the test class WebSocketEventListenerTest to
WebSocketEventListenerTests and update the corresponding Java filename and any
references to follow the required [ClassName]Tests naming convention.
Source: Path instructions
Comment on lines
+49
to
+115
| @Test | ||
| @DisplayName("연결 시 인증된 사용자면 markOnline 호출") | ||
| void onConnected_인증된사용자_markOnline() { | ||
| // given | ||
| Principal principal = accessTokenPrincipal(TokenScope.APP, 1L); | ||
| SessionConnectedEvent event = new SessionConnectedEvent(this, message, principal); | ||
|
|
||
| // when | ||
| webSocketEventListener.onConnected(event); | ||
|
|
||
| // then | ||
| then(chatPresenceStore).should().markOnline(TokenScope.APP, 1L); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("연결 시 principal이 없으면 markOnline 호출 안함") | ||
| void onConnected_principal없음_markOnline호출안함() { | ||
| // given | ||
| SessionConnectedEvent event = new SessionConnectedEvent(this, message, null); | ||
|
|
||
| // when | ||
| webSocketEventListener.onConnected(event); | ||
|
|
||
| // then | ||
| then(chatPresenceStore).should(never()).markOnline(any(), any()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("연결 시 LoginUserDto가 아닌 principal이면 markOnline 호출 안함") | ||
| void onConnected_알수없는principal_markOnline호출안함() { | ||
| // given | ||
| Principal principal = new TestingAuthenticationToken("foo", "bar"); | ||
| SessionConnectedEvent event = new SessionConnectedEvent(this, message, principal); | ||
|
|
||
| // when | ||
| webSocketEventListener.onConnected(event); | ||
|
|
||
| // then | ||
| then(chatPresenceStore).should(never()).markOnline(any(), any()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("연결 해제 시 인증된 사용자면 markOffline 호출") | ||
| void onDisconnect_인증된사용자_markOffline() { | ||
| // given | ||
| Principal principal = accessTokenPrincipal(TokenScope.MANAGER, 2L); | ||
| SessionDisconnectEvent event = new SessionDisconnectEvent(this, message, "session-1", CloseStatus.NORMAL, principal); | ||
|
|
||
| // when | ||
| webSocketEventListener.onDisconnect(event); | ||
|
|
||
| // then | ||
| then(chatPresenceStore).should().markOffline(TokenScope.MANAGER, 2L); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("연결 해제 시 principal이 없으면 markOffline 호출 안함") | ||
| void onDisconnect_principal없음_markOffline호출안함() { | ||
| // given | ||
| SessionDisconnectEvent event = new SessionDisconnectEvent(this, message, "session-1", CloseStatus.NORMAL, null); | ||
|
|
||
| // when | ||
| webSocketEventListener.onDisconnect(event); | ||
|
|
||
| // then | ||
| then(chatPresenceStore).should(never()).markOffline(any(), any()); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
엣지 케이스 커버리지가 양호합니다.
인증된 사용자, principal 없음, 알 수 없는 principal 타입 등 다양한 시나리오를 잘 커버하고 있습니다. 다만 @Nested 클래스로 onConnected / onDisconnect 그룹핑을 추가하면 가독성이 더 향상됩니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/dreamteam/alter/common/config/WebSocketEventListenerTest.java`
around lines 49 - 115, Group the WebSocket event tests into `@Nested` classes for
readability: place the onConnected_* tests under a nested class named
OnConnected and the onDisconnect_* tests under OnDisconnect, preserving all
existing assertions and test behavior.
Source: Path instructions
Comment on lines
+8
to
+17
| class ChatRoomTest { | ||
|
|
||
| @Test | ||
| void createGroup_업장_그룹방_생성() { | ||
| ChatRoom room = ChatRoom.createGroup(100L); | ||
|
|
||
| assertThat(room.getType()).isEqualTo(ChatRoomType.GROUP); | ||
| assertThat(room.getWorkspaceId()).isEqualTo(100L); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
테스트 클래스 명명 규칙 및 BDD 패턴 위반.
경로 명령에 따르면 테스트 클래스는 [ClassName]Tests.java (복수 "Tests") 명명 규칙을 따라야 합니다. 현재 파일명은 ChatRoomTest.java (단수)이므로 ChatRoomTests.java로 변경해야 합니다. 또한 Given-When-Then 패턴의 명시적 주석과 @DisplayName 어노테이션이 누락되어 있습니다.
As per path instructions, "Test classes follow [ClassName]Tests.java naming (plural "Tests")", "Given-When-Then pattern with explicit comments", "@Nested classes group related tests with @DisplayName".
🛠️ 제안하는 수정
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
-class ChatRoomTest {
+class ChatRoomTests {
`@Test`
+ `@DisplayName`("createGroup: 업장 그룹방 생성 시 type=GROUP, workspaceId가 설정된다")
void createGroup_업장_그룹방_생성() {
+ // given
ChatRoom room = ChatRoom.createGroup(100L);
+ // when & then
assertThat(room.getType()).isEqualTo(ChatRoomType.GROUP);
assertThat(room.getWorkspaceId()).isEqualTo(100L);
}📝 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.
Suggested change
| class ChatRoomTest { | |
| @Test | |
| void createGroup_업장_그룹방_생성() { | |
| ChatRoom room = ChatRoom.createGroup(100L); | |
| assertThat(room.getType()).isEqualTo(ChatRoomType.GROUP); | |
| assertThat(room.getWorkspaceId()).isEqualTo(100L); | |
| } | |
| } | |
| class ChatRoomTests { | |
| `@Test` | |
| `@DisplayName`("createGroup: 업장 그룹방 생성 시 type=GROUP, workspaceId가 설정된다") | |
| void createGroup_업장_그룹방_생성() { | |
| // given | |
| ChatRoom room = ChatRoom.createGroup(100L); | |
| // when & then | |
| assertThat(room.getType()).isEqualTo(ChatRoomType.GROUP); | |
| assertThat(room.getWorkspaceId()).isEqualTo(100L); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/java/com/dreamteam/alter/domain/chat/entity/ChatRoomTest.java`
around lines 8 - 17, Rename the test class and file from ChatRoomTest to
ChatRoomTests to follow the plural naming convention. Update the test method to
use explicit Given-When-Then comments and add an appropriate `@DisplayName`
annotation; use `@Nested` with `@DisplayName` if grouping related tests in
ChatRoomTests.
Source: Path instructions
- MarkChatRoomRead 요청 @RequestBody에 @Valid 추가(User/Manager 컨트롤러): Spec 인터페이스의 @Valid가 구현체로 병합 보장 안 돼 @NotNull(lastReadMessageId) 검증이 누락될 수 있던 문제 해소. - V7: chat_rooms(workspace_id) WHERE type='GROUP' 부분 유니크 인덱스 — 레거시 업장 동시 워커 가입 시 그룹방 중복 생성 경쟁을 DB에서 차단. - ChatMessageResponse: 클래스 @Setter → attachments 필드 한정(다른 필드는 생성자/프로젝션만).
단일성은 V7 부분 유니크 인덱스가 보증하되, 만일의 중복 데이터에도 NonUniqueResultException을 던지지 않도록 방어.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/main/java/com/dreamteam/alter/application/chat/usecase/AbstractSendChatMessageUseCase.java (2)
120-141: 🚀 Performance & Scalability | 🔵 Trivial외부 호출(WebSocket, FCM)이
@Transactional범위 내에서 실행되고 있습니다.
execute메서드는 클래스 레벨@Transactional로 인해 DB 트랜잭션이 WebSocket 전송과 FCM 발송이 완료될 때까지 열려 있습니다. FCM 응답 지연 시 트랜잭션과 DB 커넥션이 장시간 점유됩니다.
TransactionSynchronizationManager.registerSynchronization()또는@TransactionalEventListener(phase = AFTER_COMMIT)를 사용하여 외부 호출을 트랜잭션 커밋 이후로 분리하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/application/chat/usecase/AbstractSendChatMessageUseCase.java` around lines 120 - 141, 클래스 레벨 트랜잭션 안에서 WebSocket과 FCM 외부 호출이 실행되지 않도록 수정하세요. AbstractSendChatMessageUseCase의 execute 흐름에서 메시지 저장 및 필요한 데이터를 먼저 커밋한 뒤, TransactionSynchronizationManager.registerSynchronization() 또는 `@TransactionalEventListener`(phase = AFTER_COMMIT)를 통해 WebSocket 전송과 sendDirectFcmNotification/sendGroupFcmNotification을 커밋 이후 실행하도록 분리하세요.
122-131: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win메시지
type을 브로드캐스트/응답에 포함하세요.
ChatMessageResponse와ChatMessageResponseDto모두type이 없어서NORMAL과NOTICE를 구분할 수 없습니다.ChatMessageResponse생성자와ChatMessageResponseDto.from(...)매핑에savedMessage.getType()을 반영하도록 맞춰야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/application/chat/usecase/AbstractSendChatMessageUseCase.java` around lines 122 - 131, 메시지 응답에 type이 누락되어 NORMAL과 NOTICE를 구분할 수 없습니다. AbstractSendChatMessageUseCase의 ChatMessageResponse 생성자 호출에 savedMessage.getType()을 전달하고, ChatMessageResponse 및 ChatMessageResponseDto.from(...)의 필드·생성자·매핑을 추가해 브로드캐스트와 응답 모두 type을 포함하도록 수정하세요.src/test/java/com/dreamteam/alter/application/workspace/usecase/UpdateWorkspaceRequestStatusTests.java (1)
179-207: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
workspaceId도 함께 검증하세요
workspaceRepository.save()가 스텁되지 않아Workspace.id가 null인 상태로 이벤트가 발행될 수 있습니다.memberId/scope뿐 아니라workspaceId까지 확인하거나, 저장 결과에 ID를 부여하도록 스텁해야ChatMembershipJoinedEvent와 이후 동기화 흐름을 제대로 보장할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/workspace/usecase/UpdateWorkspaceRequestStatusTests.java` around lines 179 - 207, Update execute_ACTIVATED_채팅멤버십join이벤트발행 to stub workspaceRepository.save(any(Workspace.class)) so it returns a Workspace with a non-null ID, then assert the captured ChatMembershipJoinedEvent workspaceId matches that ID in addition to memberId and scope.src/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTest.java (2)
101-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win두 번째 테스트도 BDD 스타일로 통일해 주세요.
when().thenReturn()과verify()를given().willReturn()과then().should()로 변경해 일관성을 맞춰 주세요.As per path instructions, "BDD-style Mockito: given() for setup, then().should() for verification."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTest.java` around lines 101 - 129, Update resign_정리대상없음_퇴직처리 to use BDD Mockito consistently: replace all when(...).thenReturn(...) stubbing with given(...).willReturn(...), and replace verify(...) assertions with then(...).should(...), preserving invocation counts and captured event assertions.Source: Path instructions
56-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBDD 스타일:
when().thenReturn()및verify()대신given().willReturn()및then().should()사용 권장경로 지침에 따르면 BDD 스타일 Mockito를 사용해야 합니다. 현재 첫 번째 테스트에서
when().thenReturn()으로 setup하고verify()로 검증하고 있습니다.given().willReturn()과then().should()를 사용해 일관성을 맞춰 주세요.♻️ 제안 수정 (첫 번째 테스트)
- when(worker.getId()).thenReturn(10L); + given(worker.getId()).willReturn(10L); - when(workspace.getId()).thenReturn(100L); + given(workspace.getId()).willReturn(100L); - when(user.getId()).thenReturn(200L); + given(user.getId()).willReturn(200L); - when(worker.getWorkspace()).thenReturn(workspace); + given(worker.getWorkspace()).willReturn(workspace); - when(worker.getUser()).thenReturn(user); + given(worker.getUser()).willReturn(user); - when(workspaceShiftQueryRepository.findFutureShiftsByAssignedWorker(any(), any(LocalDateTime.class))) - .thenReturn(List.of(futureShift)); + given(workspaceShiftQueryRepository.findFutureShiftsByAssignedWorker(any(), any(LocalDateTime.class))) + .willReturn(List.of(futureShift)); - when(workspaceWorkerScheduleQueryRepository.getByWorkspaceWorker(worker)) - .thenReturn(List.of(fixedSchedule)); + given(workspaceWorkerScheduleQueryRepository.getByWorkspaceWorker(worker)) + .willReturn(List.of(fixedSchedule)); - when(substituteRequestQueryRepository.findAllActiveByRequesterWorkerId(10L)) - .thenReturn(List.of(requesterRequest)); + given(substituteRequestQueryRepository.findAllActiveByRequesterWorkerId(10L)) + .willReturn(List.of(requesterRequest)); - when(substituteRequestQueryRepository.findAllPendingTargetRequestsByTargetWorkerId(10L)) - .thenReturn(List.of(targetRequest)); + given(substituteRequestQueryRepository.findAllPendingTargetRequestsByTargetWorkerId(10L)) + .willReturn(List.of(targetRequest)); - verify(futureShift, times(1)).unassignWorker(); + then(futureShift).should().unassignWorker(); - verify(fixedSchedule, times(1)).delete(); + then(fixedSchedule).should().delete(); - verify(requesterRequest, times(1)).cancel(); + then(requesterRequest).should().cancel(); - verify(targetRequest, times(1)).cancelPendingTargetAndCancelIfNoPendingTargets(10L); + then(targetRequest).should().cancelPendingTargetAndCancelIfNoPendingTargets(10L); - verify(worker, times(1)).resign(); + then(worker).should().resign(); - verify(eventPublisher, times(1)).publishEvent(eventCaptor.capture()); + then(eventPublisher).should().publishEvent(eventCaptor.capture());As per path instructions, "BDD-style Mockito: given() for setup, then().should() for verification."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTest.java` around lines 56 - 98, WorkerResignationServiceImplTest의 resign_정리대상존재_일괄정리 테스트에서 Mockito 사용을 BDD 스타일로 통일하세요. 모든 when(...).thenReturn(...) 설정을 given(...).willReturn(...)으로 변경하고, 모든 verify(...) 검증을 then(...).should(...) 형태로 변환하되 검증 횟수와 인자는 그대로 유지하세요.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/resources/db/migration/V7__unique_workspace_group_chat_room.sql`:
- Around line 4-6: 프로덕션에서 대규모 chat_rooms 테이블에 대한 쓰기 잠금을 줄이도록 마이그레이션 전략을 조정하세요.
V7의 uq_chat_group_room_per_workspace 생성에 CREATE UNIQUE INDEX CONCURRENTLY를 사용하고,
해당 마이그레이션이 트랜잭션 없이 실행되도록 Flyway의 executeInTransaction=false를 설정하거나, 데이터 규모가 작아
현재 방식을 유지해도 되는지 명시적으로 확인하세요.
In
`@src/test/java/com/dreamteam/alter/application/chat/event/ChatMembershipSyncEventListenerTest.java`:
- Line 18: Rename the test class ChatMembershipSyncEventListenerTest to
ChatMembershipSyncEventListenerTests and update the corresponding filename and
any references to follow the required [ClassName]Tests naming convention.
In
`@src/test/java/com/dreamteam/alter/application/chat/usecase/CreateOrGetChatRoomTest.java`:
- Around line 46-101: 자기 자신과 동일한 사용자·스코프로 채팅방 생성을 요청할 때 검증 예외가 발생하는 테스트를 추가하세요.
CreateOrGetChatRoomTest의 기존 execute 테스트들과 동일한 방식으로 actor와 상대방 ID 및
TokenScope.APP를 설정하고, sut.execute(actor, 동일한 사용자 ID, TokenScope.APP)이 적절한 예외를
던지는지 검증하며 chatRoomRepository.save와 chatRoomMemberRepository.saveAll이 호출되지 않았는지도
확인하세요.
In
`@src/test/java/com/dreamteam/alter/application/chat/usecase/SendChatMessageTest.java`:
- Around line 88-489: Group the related test methods in SendChatMessageTest into
nested test classes annotated with `@Nested` and descriptive `@DisplayName` values.
Organize them into logical groups such as NOT_FOUND/permission validation,
DIRECT message sending, GROUP message sending, and attachment handling,
following the JoinTests pattern used elsewhere in the project.
---
Outside diff comments:
In
`@src/main/java/com/dreamteam/alter/application/chat/usecase/AbstractSendChatMessageUseCase.java`:
- Around line 120-141: 클래스 레벨 트랜잭션 안에서 WebSocket과 FCM 외부 호출이 실행되지 않도록 수정하세요.
AbstractSendChatMessageUseCase의 execute 흐름에서 메시지 저장 및 필요한 데이터를 먼저 커밋한 뒤,
TransactionSynchronizationManager.registerSynchronization() 또는
`@TransactionalEventListener`(phase = AFTER_COMMIT)를 통해 WebSocket 전송과
sendDirectFcmNotification/sendGroupFcmNotification을 커밋 이후 실행하도록 분리하세요.
- Around line 122-131: 메시지 응답에 type이 누락되어 NORMAL과 NOTICE를 구분할 수 없습니다.
AbstractSendChatMessageUseCase의 ChatMessageResponse 생성자 호출에
savedMessage.getType()을 전달하고, ChatMessageResponse 및
ChatMessageResponseDto.from(...)의 필드·생성자·매핑을 추가해 브로드캐스트와 응답 모두 type을 포함하도록
수정하세요.
In
`@src/test/java/com/dreamteam/alter/application/workspace/usecase/UpdateWorkspaceRequestStatusTests.java`:
- Around line 179-207: Update execute_ACTIVATED_채팅멤버십join이벤트발행 to stub
workspaceRepository.save(any(Workspace.class)) so it returns a Workspace with a
non-null ID, then assert the captured ChatMembershipJoinedEvent workspaceId
matches that ID in addition to memberId and scope.
In
`@src/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTest.java`:
- Around line 101-129: Update resign_정리대상없음_퇴직처리 to use BDD Mockito
consistently: replace all when(...).thenReturn(...) stubbing with
given(...).willReturn(...), and replace verify(...) assertions with
then(...).should(...), preserving invocation counts and captured event
assertions.
- Around line 56-98: WorkerResignationServiceImplTest의 resign_정리대상존재_일괄정리 테스트에서
Mockito 사용을 BDD 스타일로 통일하세요. 모든 when(...).thenReturn(...) 설정을
given(...).willReturn(...)으로 변경하고, 모든 verify(...) 검증을 then(...).should(...) 형태로
변환하되 검증 횟수와 인자는 그대로 유지하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dbb9211c-2d0f-40d2-a41b-2ca33de4ef20
📒 Files selected for processing (55)
build.gradlesrc/main/java/com/dreamteam/alter/adapter/inbound/general/chat/controller/UserChatController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/chat/controller/UserChatControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/chat/dto/ChatMessageResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/chat/dto/MarkChatRoomReadRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/chat/dto/SendChatMessageRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/chat/controller/ManagerChatController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/chat/controller/ManagerChatControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/outbound/chat/persistence/ChatRoomQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/chat/persistence/readonly/ChatMessageResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/chat/event/ChatMembershipJoinedEvent.javasrc/main/java/com/dreamteam/alter/application/chat/event/ChatMembershipLeftEvent.javasrc/main/java/com/dreamteam/alter/application/chat/event/ChatMembershipSyncEventListener.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/AbstractCreateOrGetChatRoomUseCase.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/AbstractGetChatMessagesUseCase.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/AbstractSendChatMessageUseCase.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/CreateOrGetChatRoom.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/GetChatMessages.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/GetWorkspaceGroupChatRoom.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/ManagerCreateOrGetChatRoom.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/ManagerGetChatMessages.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/ManagerSendChatMessage.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/MarkChatRoomRead.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/SendChatMessage.javasrc/main/java/com/dreamteam/alter/application/chat/usecase/SyncWorkspaceChatMembership.javasrc/main/java/com/dreamteam/alter/application/workspace/usecase/AddWorkerToWorkspace.javasrc/main/java/com/dreamteam/alter/application/workspace/usecase/UpdateWorkspaceRequestStatus.javasrc/main/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImpl.javasrc/main/java/com/dreamteam/alter/common/config/WebSocketConfig.javasrc/main/java/com/dreamteam/alter/common/config/WebSocketEventListener.javasrc/main/java/com/dreamteam/alter/common/exception/ErrorCode.javasrc/main/java/com/dreamteam/alter/common/notification/NotificationMessageConstants.javasrc/main/java/com/dreamteam/alter/domain/chat/entity/ChatMessage.javasrc/main/java/com/dreamteam/alter/domain/chat/port/inbound/GetWorkspaceGroupChatRoomUseCase.javasrc/main/java/com/dreamteam/alter/domain/chat/port/inbound/MarkChatRoomReadUseCase.javasrc/main/java/com/dreamteam/alter/domain/chat/port/outbound/ChatPresenceStore.javasrc/main/resources/application.ymlsrc/main/resources/db/migration/V5__chat_message_content_nullable.sqlsrc/main/resources/db/migration/V6__backfill_workspace_group_chat.sqlsrc/main/resources/db/migration/V7__unique_workspace_group_chat_room.sqlsrc/test/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImplTest.javasrc/test/java/com/dreamteam/alter/application/chat/event/ChatMembershipSyncEventListenerTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/CreateOrGetChatRoomTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/GetChatMessagesTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/GetWorkspaceGroupChatRoomTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/ManagerSendChatMessageTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/MarkChatRoomReadTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/SendChatMessageTest.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/SyncWorkspaceChatMembershipTest.javasrc/test/java/com/dreamteam/alter/application/workspace/usecase/AddWorkerToWorkspaceTest.javasrc/test/java/com/dreamteam/alter/application/workspace/usecase/UpdateWorkspaceRequestStatusTests.javasrc/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTest.javasrc/test/java/com/dreamteam/alter/common/config/WebSocketEventListenerTest.javasrc/test/resources/application.yml
Comment on lines
+4
to
+6
| CREATE UNIQUE INDEX uq_chat_group_room_per_workspace | ||
| ON chat_rooms (workspace_id) | ||
| WHERE type = 'GROUP'; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial
프로덕션 환경에서 CONCURRENTLY 사용을 고려하세요.
CREATE UNIQUE INDEX(비동기)는 인덱스 생성 중 테이블 쓰기를 차단합니다. V6 백필 이후 chat_rooms 행 수가 많다면 인덱스 생성 시간 동안 쓰기 락이 지속될 수 있습니다.
Flyway는 기본적으로 마이그레이션을 트랜잭션 내에서 실행하므로 CONCURRENTLY를 직접 사용할 수 없습니다. 대안:
- 해당 마이그레이션에
executeInTransaction=false설정 (Flyway config 또는@Baseline활용) - 또는 V6 백필 시점에
chat_rooms행 수가 적어 락 시간이 무시 가능한 경우 현재 방식 유지
🧰 Tools
🪛 Squawk (2.59.0)
[warning] 4-6: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.
(require-concurrent-index-creation)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/resources/db/migration/V7__unique_workspace_group_chat_room.sql`
around lines 4 - 6, 프로덕션에서 대규모 chat_rooms 테이블에 대한 쓰기 잠금을 줄이도록 마이그레이션 전략을 조정하세요.
V7의 uq_chat_group_room_per_workspace 생성에 CREATE UNIQUE INDEX CONCURRENTLY를 사용하고,
해당 마이그레이션이 트랜잭션 없이 실행되도록 Flyway의 executeInTransaction=false를 설정하거나, 데이터 규모가 작아
현재 방식을 유지해도 되는지 명시적으로 확인하세요.
Source: Linters/SAST tools
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| @DisplayName("ChatMembershipSyncEventListener 테스트") | ||
| class ChatMembershipSyncEventListenerTest { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
테스트 클래스 명명 규칙 위반: Test → Tests
경로 지침에 따르면 테스트 클래스는 [ClassName]Tests.java 명명 규칙(복수형 "Tests")을 따라야 합니다. ChatMembershipSyncEventListenerTest → ChatMembershipSyncEventListenerTests로 변경해 주세요.
As per path instructions, "Test classes follow [ClassName]Tests.java naming (plural "Tests")."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/dreamteam/alter/application/chat/event/ChatMembershipSyncEventListenerTest.java`
at line 18, Rename the test class ChatMembershipSyncEventListenerTest to
ChatMembershipSyncEventListenerTests and update the corresponding filename and
any references to follow the required [ClassName]Tests naming convention.
Source: Path instructions
Comment on lines
+46
to
+101
| @Test | ||
| @DisplayName("기존 채팅방이 없으면 새로 생성하고 참여자 2명의 chat_room_members 행을 생성한다") | ||
| void execute_신규방생성_멤버행2개생성() { | ||
| // given | ||
| User user = mock(User.class); | ||
| given(user.getId()).willReturn(1L); | ||
| AppActor actor = AppActor.from(user, List.of()); | ||
|
|
||
| given(chatRoomQueryRepository.findExistingChatRoom(1L, TokenScope.APP, 2L, TokenScope.APP)) | ||
| .willReturn(Optional.empty()); | ||
|
|
||
| ChatRoom savedRoom = mock(ChatRoom.class); | ||
| given(savedRoom.getId()).willReturn(99L); | ||
| given(chatRoomRepository.save(any(ChatRoom.class))).willReturn(savedRoom); | ||
|
|
||
| // when | ||
| CreateChatRoomResponseDto response = sut.execute(actor, 2L, TokenScope.APP); | ||
|
|
||
| // then | ||
| assertThat(response.getChatRoomId()).isEqualTo(99L); | ||
|
|
||
| ArgumentCaptor<List<ChatRoomMember>> captor = ArgumentCaptor.forClass(List.class); | ||
| then(chatRoomMemberRepository).should().saveAll(captor.capture()); | ||
| List<ChatRoomMember> savedMembers = captor.getValue(); | ||
|
|
||
| assertThat(savedMembers).hasSize(2); | ||
| assertThat(savedMembers) | ||
| .extracting(ChatRoomMember::getChatRoomId, ChatRoomMember::getMemberId, ChatRoomMember::getMemberScope) | ||
| .containsExactlyInAnyOrder( | ||
| org.assertj.core.api.Assertions.tuple(99L, 1L, TokenScope.APP), | ||
| org.assertj.core.api.Assertions.tuple(99L, 2L, TokenScope.APP) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("기존 채팅방이 있으면 재사용하고 chat_room_members 행을 생성하지 않는다") | ||
| void execute_기존방재사용_멤버행미생성() { | ||
| // given | ||
| User user = mock(User.class); | ||
| given(user.getId()).willReturn(1L); | ||
| AppActor actor = AppActor.from(user, List.of()); | ||
|
|
||
| ChatRoom existingRoom = mock(ChatRoom.class); | ||
| given(existingRoom.getId()).willReturn(42L); | ||
| given(chatRoomQueryRepository.findExistingChatRoom(1L, TokenScope.APP, 2L, TokenScope.APP)) | ||
| .willReturn(Optional.of(existingRoom)); | ||
|
|
||
| // when | ||
| CreateChatRoomResponseDto response = sut.execute(actor, 2L, TokenScope.APP); | ||
|
|
||
| // then | ||
| assertThat(response.getChatRoomId()).isEqualTo(42L); | ||
| then(chatRoomRepository).should(never()).save(any()); | ||
| then(chatRoomMemberRepository).should(never()).saveAll(any()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
자기 자신과의 채팅방 생성 방지 테스트가 누락되었습니다.
AbstractCreateOrGetChatRoomUseCase의 execute() 메서드(라인 30-32)에는 currentUserId.equals(opponentUserId) && currentScope == opponentScope 조건으로 자기 자신과의 채팅방 생성을 방지하는 검증 로직이 있습니다. 이 분기에 대한 테스트 케이스가 추가되어야 합니다.
💚 제안하는 테스트 추가
`@Test`
+ `@DisplayName`("자기 자신과는 채팅방 생성 시 ILLEGAL_ARGUMENT 예외가 발생한다")
+ void execute_자기자신_채팅방생성_예외() {
+ // given
+ User user = mock(User.class);
+ given(user.getId()).willReturn(1L);
+ AppActor actor = AppActor.from(user, List.of());
+
+ // when & then
+ assertThatThrownBy(() -> sut.execute(actor, 1L, TokenScope.APP))
+ .isInstanceOf(CustomException.class)
+ .hasFieldOrPropertyWithValue("errorCode", ErrorCode.ILLEGAL_ARGUMENT);
+
+ then(chatRoomQueryRepository).should(never()).findExistingChatRoom(any(), any(), any(), any());
+ then(chatRoomRepository).should(never()).save(any());
+ }
+
+ `@Test`
`@DisplayName`("기존 채팅방이 없으면 새로 생성하고 참여자 2명의 chat_room_members 행을 생성한다")
void execute_신규방생성_멤버행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.
Suggested change
| @Test | |
| @DisplayName("기존 채팅방이 없으면 새로 생성하고 참여자 2명의 chat_room_members 행을 생성한다") | |
| void execute_신규방생성_멤버행2개생성() { | |
| // given | |
| User user = mock(User.class); | |
| given(user.getId()).willReturn(1L); | |
| AppActor actor = AppActor.from(user, List.of()); | |
| given(chatRoomQueryRepository.findExistingChatRoom(1L, TokenScope.APP, 2L, TokenScope.APP)) | |
| .willReturn(Optional.empty()); | |
| ChatRoom savedRoom = mock(ChatRoom.class); | |
| given(savedRoom.getId()).willReturn(99L); | |
| given(chatRoomRepository.save(any(ChatRoom.class))).willReturn(savedRoom); | |
| // when | |
| CreateChatRoomResponseDto response = sut.execute(actor, 2L, TokenScope.APP); | |
| // then | |
| assertThat(response.getChatRoomId()).isEqualTo(99L); | |
| ArgumentCaptor<List<ChatRoomMember>> captor = ArgumentCaptor.forClass(List.class); | |
| then(chatRoomMemberRepository).should().saveAll(captor.capture()); | |
| List<ChatRoomMember> savedMembers = captor.getValue(); | |
| assertThat(savedMembers).hasSize(2); | |
| assertThat(savedMembers) | |
| .extracting(ChatRoomMember::getChatRoomId, ChatRoomMember::getMemberId, ChatRoomMember::getMemberScope) | |
| .containsExactlyInAnyOrder( | |
| org.assertj.core.api.Assertions.tuple(99L, 1L, TokenScope.APP), | |
| org.assertj.core.api.Assertions.tuple(99L, 2L, TokenScope.APP) | |
| ); | |
| } | |
| @Test | |
| @DisplayName("기존 채팅방이 있으면 재사용하고 chat_room_members 행을 생성하지 않는다") | |
| void execute_기존방재사용_멤버행미생성() { | |
| // given | |
| User user = mock(User.class); | |
| given(user.getId()).willReturn(1L); | |
| AppActor actor = AppActor.from(user, List.of()); | |
| ChatRoom existingRoom = mock(ChatRoom.class); | |
| given(existingRoom.getId()).willReturn(42L); | |
| given(chatRoomQueryRepository.findExistingChatRoom(1L, TokenScope.APP, 2L, TokenScope.APP)) | |
| .willReturn(Optional.of(existingRoom)); | |
| // when | |
| CreateChatRoomResponseDto response = sut.execute(actor, 2L, TokenScope.APP); | |
| // then | |
| assertThat(response.getChatRoomId()).isEqualTo(42L); | |
| then(chatRoomRepository).should(never()).save(any()); | |
| then(chatRoomMemberRepository).should(never()).saveAll(any()); | |
| } | |
| } | |
| `@Test` | |
| `@DisplayName`("자기 자신과는 채팅방 생성 시 ILLEGAL_ARGUMENT 예외가 발생한다") | |
| void execute_자기자신_채팅방생성_예외() { | |
| // given | |
| User user = mock(User.class); | |
| given(user.getId()).willReturn(1L); | |
| AppActor actor = AppActor.from(user, List.of()); | |
| // when & then | |
| assertThatThrownBy(() -> sut.execute(actor, 1L, TokenScope.APP)) | |
| .isInstanceOf(CustomException.class) | |
| .hasFieldOrPropertyWithValue("errorCode", ErrorCode.ILLEGAL_ARGUMENT); | |
| then(chatRoomQueryRepository).should(never()).findExistingChatRoom(any(), any(), any(), any()); | |
| then(chatRoomRepository).should(never()).save(any()); | |
| } | |
| `@Test` | |
| `@DisplayName`("기존 채팅방이 없으면 새로 생성하고 참여자 2명의 chat_room_members 행을 생성한다") | |
| void execute_신규방생성_멤버행2개생성() { | |
| // given | |
| User user = mock(User.class); | |
| given(user.getId()).willReturn(1L); | |
| AppActor actor = AppActor.from(user, List.of()); | |
| given(chatRoomQueryRepository.findExistingChatRoom(1L, TokenScope.APP, 2L, TokenScope.APP)) | |
| .willReturn(Optional.empty()); | |
| ChatRoom savedRoom = mock(ChatRoom.class); | |
| given(savedRoom.getId()).willReturn(99L); | |
| given(chatRoomRepository.save(any(ChatRoom.class))).willReturn(savedRoom); | |
| // when | |
| CreateChatRoomResponseDto response = sut.execute(actor, 2L, TokenScope.APP); | |
| // then | |
| assertThat(response.getChatRoomId()).isEqualTo(99L); | |
| ArgumentCaptor<List<ChatRoomMember>> captor = ArgumentCaptor.forClass(List.class); | |
| then(chatRoomMemberRepository).should().saveAll(captor.capture()); | |
| List<ChatRoomMember> savedMembers = captor.getValue(); | |
| assertThat(savedMembers).hasSize(2); | |
| assertThat(savedMembers) | |
| .extracting(ChatRoomMember::getChatRoomId, ChatRoomMember::getMemberId, ChatRoomMember::getMemberScope) | |
| .containsExactlyInAnyOrder( | |
| org.assertj.core.api.Assertions.tuple(99L, 1L, TokenScope.APP), | |
| org.assertj.core.api.Assertions.tuple(99L, 2L, TokenScope.APP) | |
| ); | |
| } | |
| `@Test` | |
| `@DisplayName`("기존 채팅방이 있으면 재사용하고 chat_room_members 행을 생성하지 않는다") | |
| void execute_기존방재사용_멤버행미생성() { | |
| // given | |
| User user = mock(User.class); | |
| given(user.getId()).willReturn(1L); | |
| AppActor actor = AppActor.from(user, List.of()); | |
| ChatRoom existingRoom = mock(ChatRoom.class); | |
| given(existingRoom.getId()).willReturn(42L); | |
| given(chatRoomQueryRepository.findExistingChatRoom(1L, TokenScope.APP, 2L, TokenScope.APP)) | |
| .willReturn(Optional.of(existingRoom)); | |
| // when | |
| CreateChatRoomResponseDto response = sut.execute(actor, 2L, TokenScope.APP); | |
| // then | |
| assertThat(response.getChatRoomId()).isEqualTo(42L); | |
| then(chatRoomRepository).should(never()).save(any()); | |
| then(chatRoomMemberRepository).should(never()).saveAll(any()); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/dreamteam/alter/application/chat/usecase/CreateOrGetChatRoomTest.java`
around lines 46 - 101, 자기 자신과 동일한 사용자·스코프로 채팅방 생성을 요청할 때 검증 예외가 발생하는 테스트를 추가하세요.
CreateOrGetChatRoomTest의 기존 execute 테스트들과 동일한 방식으로 actor와 상대방 ID 및
TokenScope.APP를 설정하고, sut.execute(actor, 동일한 사용자 ID, TokenScope.APP)이 적절한 예외를
던지는지 검증하며 chatRoomRepository.save와 chatRoomMemberRepository.saveAll이 호출되지 않았는지도
확인하세요.
Comment on lines
+88
to
+489
| sut.execute(user, request, 100L); | ||
|
|
||
| // then | ||
| ArgumentCaptor<String> bodyCaptor = ArgumentCaptor.forClass(String.class); | ||
| then(notificationService).should().sendNotificationOnly(eq(2L), any(), anyString(), bodyCaptor.capture()); | ||
| assertThat(bodyCaptor.getValue()).isEqualTo("홍길동: 사진을 보냈습니다"); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("텍스트 메시지 전송시 FCM 알림 본문은 기존 텍스트 미리보기 유지") | ||
| void execute_텍스트_메시지면_FCM_본문_기존유지() { | ||
| // given | ||
| User user = mock(User.class); | ||
| given(user.getId()).willReturn(1L); | ||
|
|
||
| ChatRoom directRoom = mock(ChatRoom.class); | ||
| given(directRoom.getId()).willReturn(100L); | ||
| given(directRoom.getType()).willReturn(ChatRoomType.DIRECT); | ||
| given(directRoom.getParticipant1Id()).willReturn(1L); | ||
| given(directRoom.getParticipant1Scope()).willReturn(TokenScope.APP); | ||
| given(directRoom.getParticipant2Id()).willReturn(2L); | ||
| given(directRoom.getParticipant2Scope()).willReturn(TokenScope.APP); | ||
|
|
||
| given(chatRoomQueryRepository.findById(100L)).willReturn(Optional.of(directRoom)); | ||
| given(chatRoomQueryRepository.findByIdAndParticipant(100L, 1L, TokenScope.APP)) | ||
| .willReturn(Optional.of(directRoom)); | ||
|
|
||
| SendChatMessageRequestDto request = mock(SendChatMessageRequestDto.class); | ||
| given(request.getContent()).willReturn("안녕하세요"); | ||
|
|
||
| ChatMessage savedMessage = ChatMessage.create(100L, 1L, TokenScope.APP, ChatMessageType.NORMAL, "안녕하세요"); | ||
| given(chatMessageRepository.save(any(ChatMessage.class))).willReturn(savedMessage); | ||
|
|
||
| User sender = mock(User.class); | ||
| given(sender.getName()).willReturn("홍길동"); | ||
| given(userQueryRepository.findById(1L)).willReturn(Optional.of(sender)); | ||
| given(chatPresenceStore.isOnline(TokenScope.APP, 2L)).willReturn(false); | ||
|
|
||
| // when | ||
| sut.execute(user, request, 100L); | ||
|
|
||
| // then | ||
| ArgumentCaptor<String> bodyCaptor = ArgumentCaptor.forClass(String.class); | ||
| then(notificationService).should().sendNotificationOnly(eq(2L), any(), anyString(), bodyCaptor.capture()); | ||
| assertThat(bodyCaptor.getValue()).isEqualTo("홍길동: 안녕하세요"); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("이미지 첨부시 attachFilesUseCase 호출 및 브로드캐스트 payload에 attachments 포함") | ||
| void execute_이미지_첨부시_attach_호출_및_payload_포함() { | ||
| // given | ||
| User user = mock(User.class); | ||
| given(user.getId()).willReturn(1L); | ||
|
|
||
| ChatRoom directRoom = mock(ChatRoom.class); | ||
| given(directRoom.getId()).willReturn(100L); | ||
| given(directRoom.getType()).willReturn(ChatRoomType.DIRECT); | ||
| given(directRoom.getParticipant1Id()).willReturn(1L); | ||
| given(directRoom.getParticipant1Scope()).willReturn(TokenScope.APP); | ||
| given(directRoom.getParticipant2Id()).willReturn(2L); | ||
| given(directRoom.getParticipant2Scope()).willReturn(TokenScope.APP); | ||
|
|
||
| given(chatRoomQueryRepository.findById(100L)).willReturn(Optional.of(directRoom)); | ||
| given(chatRoomQueryRepository.findByIdAndParticipant(100L, 1L, TokenScope.APP)) | ||
| .willReturn(Optional.of(directRoom)); | ||
|
|
||
| SendChatMessageRequestDto request = mock(SendChatMessageRequestDto.class); | ||
| given(request.getContent()).willReturn(null); | ||
| given(request.getFileIds()).willReturn(List.of("f1", "f2")); | ||
|
|
||
| ChatMessage savedMessage = ChatMessage.create(100L, 1L, TokenScope.APP, ChatMessageType.NORMAL, null); | ||
| given(chatMessageRepository.save(any(ChatMessage.class))).willReturn(savedMessage); | ||
| given(userQueryRepository.findById(1L)).willReturn(Optional.empty()); | ||
| given(chatPresenceStore.isOnline(TokenScope.APP, 2L)).willReturn(false); | ||
|
|
||
| File file1 = mock(File.class); | ||
| File file2 = mock(File.class); | ||
| given(fileQueryRepository.findAllByTargetTypeAndTargetIdIn( | ||
| eq(FileTargetType.CHAT_MESSAGE), eq(List.of(String.valueOf(savedMessage.getId()))) | ||
| )).willReturn(List.of(file1, file2)); | ||
|
|
||
| FileResponseDto dto1 = FileResponseDto.of(file1, "https://cdn.example.com/f1"); | ||
| FileResponseDto dto2 = FileResponseDto.of(file2, "https://cdn.example.com/f2"); | ||
| given(fileUrlService.resolve(file1)).willReturn(dto1); | ||
| given(fileUrlService.resolve(file2)).willReturn(dto2); | ||
|
|
||
| // when | ||
| sut.execute(user, request, 100L); | ||
|
|
||
| // then | ||
| then(attachFilesUseCase).should().execute( | ||
| eq(List.of("f1", "f2")), eq(FileTargetType.CHAT_MESSAGE), anyString(), eq(1L) | ||
| ); | ||
|
|
||
| ArgumentCaptor<Object> captor = ArgumentCaptor.forClass(Object.class); | ||
| then(messagingTemplate).should().convertAndSend(eq("/sub/chat.100"), captor.capture()); | ||
| ChatMessageResponse payload = (ChatMessageResponse) captor.getValue(); | ||
| assertThat(payload.getAttachments()).containsExactly(dto1, dto2); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
관련 테스트를 @Nested 그룹으로 묶는 것을 권장
현재 12개의 테스트 메서드가 @Nested 그룹화 없이 나열되어 있습니다. NOT_FOUND/권한 검증, DIRECT 전송, GROUP 전송, 첨부파일 케이스 등으로 @Nested + @DisplayName을 활용해 그룹화하면 (예: SyncWorkspaceChatMembershipTest의 JoinTests처럼) 가독성과 프로젝트 컨벤션 일관성이 향상됩니다.
As per path instructions, "@Nested classes group related tests with @DisplayName."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/dreamteam/alter/application/chat/usecase/SendChatMessageTest.java`
around lines 88 - 489, Group the related test methods in SendChatMessageTest
into nested test classes annotated with `@Nested` and descriptive `@DisplayName`
values. Organize them into logical groups such as NOT_FOUND/permission
validation, DIRECT message sending, GROUP message sending, and attachment
handling, following the JoinTests pattern used elsewhere in the project.
Source: Path instructions
멀티탭/멀티디바이스로 접속한 사용자가 세션 하나만 닫아도 presence가
즉시 오프라인으로 바뀌어 FCM 푸시가 잘못 발송되는 문제를 수정.
per-user 단일 키 대신 Redis SET(chat:presence:sessions:{scope}:{memberId})에
세션 id를 저장해 참조 카운트를 세고, SET이 비어야(마지막 세션 종료) 오프라인이
되도록 변경. isOnline(TokenScope, Long) 시그니처는 유지해 AbstractSendChatMessageUseCase는
영향 없음. WebSocketEventListener는 StompHeaderAccessor로 세션 id를 추출해
markOnline/markOffline에 전달. refresh()는 호출부가 없어 제거.
CodeRabbit 지적(경로 지침) 반영 — 이번 PR에서 추가된 14개 테스트 클래스명·파일명을 프로젝트 컨벤션(*Tests)에 맞춤.
WorkerResignationServiceImplTests, AddWorkerToWorkspaceTests의 classic Mockito when/thenReturn 스타일을 BDD given/willReturn으로 변환하여 기존 BDD 스타일 테스트들과 통일
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTests.java (3)
88-94: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winBDD 스타일 위반:
verify()대신then().should()사용경로 지침에 따르면 검증 시
then().should()를 사용해야 합니다.verify()대신 BDD 스타일로 변경해 주세요.♻️ 제안 수정
- verify(futureShift, times(1)).unassignWorker(); - verify(fixedSchedule, times(1)).delete(); - verify(requesterRequest, times(1)).cancel(); - verify(targetRequest, times(1)).cancelPendingTargetAndCancelIfNoPendingTargets(10L); - verify(worker, times(1)).resign(); - verify(eventPublisher, times(1)).publishEvent(eventCaptor.capture()); + then(futureShift).should(times(1)).unassignWorker(); + then(fixedSchedule).should(times(1)).delete(); + then(requesterRequest).should(times(1)).cancel(); + then(targetRequest).should(times(1)).cancelPendingTargetAndCancelIfNoPendingTargets(10L); + then(worker).should(times(1)).resign(); + then(eventPublisher).should(times(1)).publishEvent(eventCaptor.capture());As per path instructions, "BDD-style Mockito: given() for setup, then().should() for verification."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTests.java` around lines 88 - 94, Replace all Mockito verify() calls in the resignation test’s verification section with BDDMockito then(...).should() calls, preserving each invocation, argument, and times(1) behavior; update the eventCaptor verification consistently.Source: Path instructions
36-36: 📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win컴파일 에러: 클래스명과 파일명 불일치
파일명은
WorkerResignationServiceImplTests.java이지만 클래스명은WorkerResignationServiceImplTest(단수)입니다. 클래스명을 복수형으로 변경해야 합니다.🔧 제안 수정
-class WorkerResignationServiceImplTest { +class WorkerResignationServiceImplTests {As per path instructions, "Test classes follow [ClassName]Tests.java naming (plural "Tests")."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTests.java` at line 36, Rename the test class WorkerResignationServiceImplTest to WorkerResignationServiceImplTests so it matches the filename and the project’s required [ClassName]Tests.java naming convention.Source: Path instructions
122-124: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win동일한 BDD 스타일 위반이 두 번째 테스트에도 존재합니다.
verify()를then().should()로 변경해 주세요.♻️ 제안 수정
- verify(worker, times(1)).resign(); - verify(eventPublisher, times(1)).publishEvent(eventCaptor.capture()); + then(worker).should(times(1)).resign(); + then(eventPublisher).should(times(1)).publishEvent(eventCaptor.capture());As per path instructions, "BDD-style Mockito: given() for setup, then().should() for verification."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTests.java` around lines 122 - 124, 두 번째 테스트의 검증이 BDD Mockito 스타일을 따르지 않습니다. 해당 테스트의 then 블록에서 worker.resign() 및 eventPublisher.publishEvent(eventCaptor.capture())에 대한 verify() 호출을 then().should() 검증으로 변경하세요.Source: Path instructions
src/test/java/com/dreamteam/alter/common/config/WebSocketEventListenerTests.java (2)
49-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBDD 스타일:
lenient().when()대신lenient().given()사용 권장헬퍼 메서드에서
lenient().when().thenReturn()을 사용하고 있습니다.lenient().given().willReturn()으로 변경해 주세요.♻️ 제안 수정
- lenient().when(dto.getScope()).thenReturn(scope); - lenient().when(dto.getId()).thenReturn(memberId); + lenient().given(dto.getScope()).willReturn(scope); + lenient().given(dto.getId()).willReturn(memberId);As per path instructions, "BDD-style Mockito: given() for setup, then().should() for verification."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/common/config/WebSocketEventListenerTests.java` around lines 49 - 50, In the helper setup for the mocked dto, replace the BDD-inconsistent lenient().when(dto.getScope()).thenReturn(scope) and lenient().when(dto.getId()).thenReturn(memberId) calls with lenient().given(...).willReturn(...) equivalents, preserving the existing stubbing behavior.Source: Path instructions
33-33: 📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win컴파일 에러: 파일명 변경 후 클래스명 미수정
이전 리뷰에서 명명 규칙 위반을 지적했고, 파일명은
WebSocketEventListenerTests.java로 변경되었으나 클래스 선언은 여전히WebSocketEventListenerTest(단수)입니다.🔧 제안 수정
-class WebSocketEventListenerTest { +class WebSocketEventListenerTests {As per path instructions, "Test classes follow [ClassName]Tests.java naming (plural "Tests")."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/common/config/WebSocketEventListenerTests.java` at line 33, WebSocketEventListenerTests.java의 클래스 선언명을 파일명과 명명 규칙에 맞게 WebSocketEventListenerTests로 변경하고, 관련 참조가 있다면 동일한 이름으로 갱신하세요.Source: Path instructions
src/test/java/com/dreamteam/alter/domain/chat/entity/ChatRoomTests.java (1)
9-23: 📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win컴파일 에러: 파일명 변경 후 클래스명 미수정 및 테스트 컨벤션 누락
이전 리뷰에서 명명 규칙,
@DisplayName, Given-When-Then 주석 누락을 지적했습니다. 파일명은ChatRoomTests.java로 변경되었으나 클래스 선언은 여전히ChatRoomTest(단수)이며,@DisplayName과 Given-When-Then 주석도 여전히 누락되어 있습니다.🔧 제안 수정
-class ChatRoomTest { +class ChatRoomTests { `@Test` + `@DisplayName`("createGroup: 업장 그룹방 생성 시 type=GROUP, workspaceId가 설정된다") void createGroup_업장_그룹방_생성() { + // given ChatRoom room = ChatRoom.createGroup(100L); + // when & then assertThat(room.getType()).isEqualTo(ChatRoomType.GROUP); assertThat(room.getWorkspaceId()).isEqualTo(100L); } `@Test` + `@DisplayName`("createGroup: workspaceId가 null이면 IllegalArgumentException을 던진다") void createGroup_workspaceId_null이면_예외() { + // given & when & then assertThatThrownBy(() -> ChatRoom.createGroup(null)) .isInstanceOf(IllegalArgumentException.class); }As per path instructions, "Test classes follow [ClassName]Tests.java naming (plural "Tests")", "Given-When-Then pattern with explicit comments", "
@Nestedclasses group related tests with@DisplayName".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/domain/chat/entity/ChatRoomTests.java` around lines 9 - 23, ChatRoomTests.java의 클래스 선언을 ChatRoomTests로 변경해 파일명과 일치시키고, createGroup 관련 테스트를 `@Nested` 그룹으로 묶어 `@DisplayName을` 추가하세요. 각 테스트에는 Given-When-Then 단계가 드러나도록 명시적 주석을 추가하고, 테스트 메서드에도 적절한 `@DisplayName을` 지정하세요.Source: Path instructions
src/test/java/com/dreamteam/alter/application/workspace/usecase/AddWorkerToWorkspaceTests.java (1)
33-33: 📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win컴파일 에러: 파일명 변경 후 클래스명 미수정
이전 리뷰에서 명명 규칙 위반을 지적했고, 파일명은
AddWorkerToWorkspaceTests.java로 변경되었으나 클래스 선언은 여전히AddWorkerToWorkspaceTest(단수)입니다. Java 컴파일러는 최상위 클래스명과 파일명이 일치해야 하므로 컴파일 에러가 발생합니다.🔧 제안 수정
-class AddWorkerToWorkspaceTest { +class AddWorkerToWorkspaceTests {As per path instructions, "Test classes follow [ClassName]Tests.java naming (plural "Tests")."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/workspace/usecase/AddWorkerToWorkspaceTests.java` at line 33, 파일명과 클래스명 불일치로 컴파일 오류가 발생하므로, AddWorkerToWorkspaceTest 클래스 선언을 파일명 규칙에 맞춰 AddWorkerToWorkspaceTests로 변경하고 관련 참조도 동일하게 갱신하세요.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImpl.java`:
- Around line 24-29: ChatPresenceStoreRepositoryImpl의 markOnline()은 연결 시점에만 TTL을
설정해 장기 세션의 presence가 만료됩니다. TTL을 충분히 늘리거나 heartbeat 호출 시 expire를 갱신하도록 구현하고, 관련
heartbeat 경로에서 markOnline() 또는 별도 TTL 갱신 메서드를 호출해 활성 WebSocket 세션 동안 키가 유지되게
수정하세요.
---
Outside diff comments:
In
`@src/test/java/com/dreamteam/alter/application/workspace/usecase/AddWorkerToWorkspaceTests.java`:
- Line 33: 파일명과 클래스명 불일치로 컴파일 오류가 발생하므로, AddWorkerToWorkspaceTest 클래스 선언을 파일명
규칙에 맞춰 AddWorkerToWorkspaceTests로 변경하고 관련 참조도 동일하게 갱신하세요.
In
`@src/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTests.java`:
- Around line 88-94: Replace all Mockito verify() calls in the resignation
test’s verification section with BDDMockito then(...).should() calls, preserving
each invocation, argument, and times(1) behavior; update the eventCaptor
verification consistently.
- Line 36: Rename the test class WorkerResignationServiceImplTest to
WorkerResignationServiceImplTests so it matches the filename and the project’s
required [ClassName]Tests.java naming convention.
- Around line 122-124: 두 번째 테스트의 검증이 BDD Mockito 스타일을 따르지 않습니다. 해당 테스트의 then
블록에서 worker.resign() 및 eventPublisher.publishEvent(eventCaptor.capture())에 대한
verify() 호출을 then().should() 검증으로 변경하세요.
In
`@src/test/java/com/dreamteam/alter/common/config/WebSocketEventListenerTests.java`:
- Around line 49-50: In the helper setup for the mocked dto, replace the
BDD-inconsistent lenient().when(dto.getScope()).thenReturn(scope) and
lenient().when(dto.getId()).thenReturn(memberId) calls with
lenient().given(...).willReturn(...) equivalents, preserving the existing
stubbing behavior.
- Line 33: WebSocketEventListenerTests.java의 클래스 선언명을 파일명과 명명 규칙에 맞게
WebSocketEventListenerTests로 변경하고, 관련 참조가 있다면 동일한 이름으로 갱신하세요.
In `@src/test/java/com/dreamteam/alter/domain/chat/entity/ChatRoomTests.java`:
- Around line 9-23: ChatRoomTests.java의 클래스 선언을 ChatRoomTests로 변경해 파일명과 일치시키고,
createGroup 관련 테스트를 `@Nested` 그룹으로 묶어 `@DisplayName을` 추가하세요. 각 테스트에는 Given-When-Then
단계가 드러나도록 명시적 주석을 추가하고, 테스트 메서드에도 적절한 `@DisplayName을` 지정하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 90fd442d-d8ec-4c65-a607-2e960327a6e2
📒 Files selected for processing (18)
src/main/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImpl.javasrc/main/java/com/dreamteam/alter/common/config/WebSocketEventListener.javasrc/main/java/com/dreamteam/alter/domain/chat/entity/ChatRoom.javasrc/main/java/com/dreamteam/alter/domain/chat/port/outbound/ChatPresenceStore.javasrc/test/java/com/dreamteam/alter/adapter/outbound/chat/persistence/ChatRoomMemberRepositoryImplTests.javasrc/test/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImplTests.javasrc/test/java/com/dreamteam/alter/application/chat/event/ChatMembershipSyncEventListenerTests.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/CreateOrGetChatRoomTests.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/GetChatMessagesTests.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/GetWorkspaceGroupChatRoomTests.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/ManagerSendChatMessageTests.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/MarkChatRoomReadTests.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/SendChatMessageTests.javasrc/test/java/com/dreamteam/alter/application/chat/usecase/SyncWorkspaceChatMembershipTests.javasrc/test/java/com/dreamteam/alter/application/workspace/usecase/AddWorkerToWorkspaceTests.javasrc/test/java/com/dreamteam/alter/application/workspace/usecase/WorkerResignationServiceImplTests.javasrc/test/java/com/dreamteam/alter/common/config/WebSocketEventListenerTests.javasrc/test/java/com/dreamteam/alter/domain/chat/entity/ChatRoomTests.java
💤 Files with no reviewable changes (9)
- src/test/java/com/dreamteam/alter/adapter/outbound/chat/persistence/ChatRoomMemberRepositoryImplTests.java
- src/test/java/com/dreamteam/alter/application/chat/usecase/MarkChatRoomReadTests.java
- src/test/java/com/dreamteam/alter/application/chat/event/ChatMembershipSyncEventListenerTests.java
- src/test/java/com/dreamteam/alter/application/chat/usecase/CreateOrGetChatRoomTests.java
- src/test/java/com/dreamteam/alter/application/chat/usecase/GetWorkspaceGroupChatRoomTests.java
- src/test/java/com/dreamteam/alter/application/chat/usecase/GetChatMessagesTests.java
- src/test/java/com/dreamteam/alter/application/chat/usecase/SendChatMessageTests.java
- src/test/java/com/dreamteam/alter/application/chat/usecase/ManagerSendChatMessageTests.java
- src/test/java/com/dreamteam/alter/application/chat/usecase/SyncWorkspaceChatMembershipTests.java
Comment on lines
+24
to
+29
| @Override | ||
| public void markOnline(TokenScope scope, Long memberId, String sessionId) { | ||
| String key = key(scope, memberId); | ||
| redisTemplate.opsForSet().add(key, sessionId); | ||
| redisTemplate.expire(key, TTL); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify no periodic TTL refresh mechanism exists elsewhere in the codebase
rg -n '`@Scheduled`|refresh.*presence|heartbeat.*presence|presence.*refresh' --type=java -g '!**/test/**' -C3Repository: alter-app/alter-backend
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the presence store implementation and its related interface/listener.
git ls-files 'src/main/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImpl.java' \
'src/main/java/com/dreamteam/alter/domain/chat/*' \
'src/main/java/com/dreamteam/alter/application/chat/*' \
'src/main/java/com/dreamteam/alter/adapter/**/WebSocketEventListener.java' \
'src/main/java/com/dreamteam/alter/adapter/**/.*.java' 2>/dev/null || true
echo '--- outline: ChatPresenceStoreRepositoryImpl ---'
ast-grep outline src/main/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImpl.java --view expanded || true
echo '--- outline: ChatPresenceStore interface candidates ---'
fd -a 'ChatPresenceStore.java|ChatPresenceStoreRepository.java|WebSocketEventListener.java' src/main/java || true
echo '--- search for presence refresh/heartbeat/session TTL related code ---'
rg -n 'markOnline|markOffline|isOnline|refresh|expire\(|TTL|heartbeat|SessionConnectedEvent|SessionDisconnectEvent|ChatPresenceStore' src/main/java -g '!**/test/**' -C 2 || trueRepository: alter-app/alter-backend
Length of output: 48701
TTL 60초로는 장기 WebSocket 세션의 presence를 유지할 수 없습니다.
markOnline()이 연결 시점에만 expire(key, TTL)을 설정하고 이후 갱신하지 않아서, 60초가 지나면 Redis 키가 만료됩니다. 그 시점부터 isOnline()이 false가 되어 아직 연결된 사용자에게도 FCM 폴백이 중복 발송됩니다. TTL을 더 길게 잡거나 heartbeat 기반 갱신을 추가하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/chat/redis/ChatPresenceStoreRepositoryImpl.java`
around lines 24 - 29, ChatPresenceStoreRepositoryImpl의 markOnline()은 연결 시점에만
TTL을 설정해 장기 세션의 presence가 만료됩니다. TTL을 충분히 늘리거나 heartbeat 호출 시 expire를 갱신하도록
구현하고, 관련 heartbeat 경로에서 markOnline() 또는 별도 TTL 갱신 메서드를 호출해 활성 WebSocket 세션 동안 키가
유지되게 수정하세요.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개요
채팅 기능 전면 개선. 기존 유저↔매니저 1:1 채팅에 업장 그룹 단톡, 읽음 처리, presence 기반 푸시, 이미지 전송을 추가하고, 브로커를 **RabbitMQ STOMP relay(토글)**로 전환해 수평 확장 기반을 마련했다.
주요 변경
1. 통합 스키마 (
ChatRoom+chat_room_members)ChatRoom에type(DIRECT|GROUP)·workspace_id추가,ChatMessage에type(NORMAL|NOTICE)추가participant1/2고정 2인 →chat_room_members조인테이블로 통합 (1:1=2행, 그룹=N행)2. 업장 그룹 단톡
WorkspaceWorker+오너 매니저 전원 자동 동기화(가입/퇴사)NOTICE(공지) 메시지는 매니저만 작성 가능GET /chat/workspace/{workspaceId}/room(discovery)3. 읽음 처리
MarkChatRoomRead(POST /chat/rooms/{roomId}/read) — 멤버별last_read_message_id4. Presence + 푸시 폴백
5. 브로커: RabbitMQ STOMP relay
chat.broker.relay.enabled토글 (기본false=SimpleBroker → 기존 동작 유지)CHAT_BROKER_RELAY_ENABLED=true+RABBITMQ_RELAY_*env + RabbitMQ(STOMP 플러그인)6. 이미지 전송
FileTargetType.CHAT_MESSAGE,AttachFilesUseCase, S3)fileIds첨부, 조회/브로드캐스트 payload에 첨부 URL, 이미지-only 메시지 허용검증
ddl-auto=validate통과 (엔티티↔스키마 드리프트 없음)BrokerAvailabilityEvent available=true)배포 주의
Summary by CodeRabbit
새로운 기능
개선 사항