feat: 채팅방 생성 기능 추가 #439
feat: 채팅방 생성 기능 추가 #439
Conversation
- 멘토링의 경우에만 mentoringId가 존재하므로 long이 아닌 Long으로 설정
Gyuhyeok99
requested review from
lsy1307,
nayonsoso,
whqtker and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
src/main/java/com/example/solidconnection/mentor/service/MentoringCommandService.java (1)
54-54: 이벤트 페이로드 필드명 의미 명확화(멘토/멘티 ‘도메인 ID’ vs ‘회원 ID’ 혼동 방지)현재 mentor.getSiteUserId()를 mentorId로 전달합니다. 읽는 사람이 “멘토 도메인 ID”로 오해할 여지가 있습니다.
1) 레코드 필드명을 mentorUserId/menteeUserId로 바꾸면 오해를 줄일 수 있습니다.
2) 해당 변경은 이벤트/핸들러/테스트 전반의 네이밍 교체가 필요합니다.[표현 개선 제안이므로 선택 사항입니다]
src/main/java/com/example/solidconnection/mentor/dto/MentoringApprovedEvent.java (1)
3-12: 이벤트 필드명 정확화로 가독성 향상(선택 사항)의미 명확성을 위해 멘토/멘티의 “사이트 사용자 ID”임을 드러내는 네이밍을 제안드립니다.
1) mentorId → mentorUserId, menteeId → menteeUserId
2) of 팩토리 메서드 시그니처도 동일하게 조정예시 diff:
public record MentoringApprovedEvent( - long mentoringId, - long mentorId, - long menteeId + long mentoringId, + long mentorUserId, + long menteeUserId ) { - public static MentoringApprovedEvent of(long mentoringId, long mentorId, long menteeId) { - return new MentoringApprovedEvent(mentoringId, mentorId, menteeId); + public static MentoringApprovedEvent of(long mentoringId, long mentorUserId, long menteeUserId) { + return new MentoringApprovedEvent(mentoringId, mentorUserId, menteeUserId); } }주의: 연쇄적으로 사용하는 곳(발행/구독/테스트) 네이밍도 함께 변경해야 합니다.
src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java (1)
173-188: (선택) 거절 케이스도 비동기 이벤트 설정에 따른 플래키성 점검거절 시 이벤트가 발행되지 않는다면 현재 단언으로 충분합니다. 다만, 구조가 변경되어도 테스트가 안정적이길 원하시면 “일정 시간 내 생성이 일어나지 않음”을 await().during()로 보장하는 방식도 고려할 수 있습니다.
예시(선택):
await().during(java.time.Duration.ofSeconds(1)) .atMost(java.time.Duration.ofSeconds(2)) .untilAsserted(() -> { Optional<ChatRoom> afterChatRoom = chatRoomRepositoryForTest.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()); assertThat(afterChatRoom).isEmpty(); });src/main/java/com/example/solidconnection/mentor/service/MentoringEventHandler.java (1)
21-23: 예외 로깅을 추가해 운영 추적성을 높이세요.
- 서비스 단에서 예외를 처리하더라도 이벤트 핸들러에서 로깅하면 트러블슈팅이 쉬워집니다.
- 필요 시 Lombok @slf4j로 간단히 추가 가능합니다.
- 비즈니스 영향 없이 오류 맥락을 기록하세요.
@RequiredArgsConstructor +@lombok.extern.slf4j.Slf4j public class MentoringEventHandler { @@ - public void handleMentoringApproved(MentoringApprovedEvent event) { - chatService.createMentoringChatRoom(event.mentoringId(), event.mentorId(), event.menteeId()); - } + public void handleMentoringApproved(MentoringApprovedEvent event) { + log.debug("MentoringApprovedEvent 수신. mentoringId={}, mentorId={}, menteeId={}", + event.mentoringId(), event.mentorId(), event.menteeId()); + chatService.createMentoringChatRoom(event.mentoringId(), event.mentorId(), event.menteeId()); + }src/test/java/com/example/solidconnection/chat/repository/ChatRoomRepositoryForTest.java (1)
28-28: 메서드 시그니처는 적절합니다만, 명확한 도메인 의도를 주석으로 남기면 좋습니다.
- 정확히 2명만 포함된 1:1 방만 조회한다는 제약을 명시하세요.
- 멘토/멘티 순서가 바뀌어도 동일한 결과가 나옴을 설명하세요.
- 추후 다자간 채팅 확장 시 테스트 의도를 보호할 수 있습니다.
src/main/java/com/example/solidconnection/chat/service/ChatService.java (1)
135-140: 쿼리 라운드트립을 줄이는 선택적 리팩터를 제안합니다.
- 양방향 연관을 모두 설정하고 cascade를 활용하면 save 호출을 한 번으로 줄일 수 있습니다.
- 성능 영향은 미미할 수 있으나 트랜잭션 로그를 단순화합니다.
- 도메인 규칙상 참가자 2명이 필수라면 생성 팩토리 활용도 고려해보세요.
예시:
ChatParticipant mentorParticipant = new ChatParticipant(mentorId, chatRoom); ChatParticipant menteeParticipant = new ChatParticipant(menteeId, chatRoom); chatRoom.getChatParticipants().add(mentorParticipant); chatRoom.getChatParticipants().add(menteeParticipant); chatRoomRepository.save(chatRoom); // cascade = ALL주의: 연관관계의 주인이 ChatParticipant라면, 양쪽 모두 설정되어 있어야 합니다.
src/main/java/com/example/solidconnection/chat/domain/ChatRoom.java (2)
37-39: 명시적 nullable 표기를 통해 의도를 드러내세요.
- @column(nullable = true) 를 명시하면 스키마 의도가 더 분명해집니다.
- 문서화 효과로 유지보수성에 도움이 됩니다.
- 기능 동작에는 변화가 없습니다.
- @Column(name = "mentoring_id") + @Column(name = "mentoring_id", nullable = true) private Long mentoringId;
51-54: 멘토링 전용 정적 팩토리 메서드로 도메인 의도를 강화해보세요.
- new ChatRoom(mentoringId, false) 대신 의미 있는 이름을 사용하면 가독성이 좋아집니다.
- 생성 시점에 1:1 룸 규칙을 캡슐화할 수 있습니다.
- 예: ChatRoom.forMentoring(mentoringId).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/example/solidconnection/chat/domain/ChatRoom.java(4 hunks)src/main/java/com/example/solidconnection/chat/repository/ChatRoomRepository.java(2 hunks)src/main/java/com/example/solidconnection/chat/service/ChatService.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentoringApprovedEvent.java(1 hunks)src/main/java/com/example/solidconnection/mentor/service/MentoringCommandService.java(3 hunks)src/main/java/com/example/solidconnection/mentor/service/MentoringEventHandler.java(1 hunks)src/main/resources/db/migration/V28__add_mentoring_id_to_chat_room.sql(1 hunks)src/test/java/com/example/solidconnection/chat/repository/ChatRoomRepositoryForTest.java(1 hunks)src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java(5 hunks)
🔇 Additional comments (3)
src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java (1)
126-149: 테스트 안정성 및 검증 강화 제안
Awaitility로 비동기 처리 안정성 확보
승인 호출 직후 즉시 조회하는 대신
await().atMost(Duration.ofSeconds(3)).untilAsserted(...)로 최종 일관성을 기다린 뒤 단언하세요.멘토링-채팅방 매핑 검증 추가
생성된ChatRoom의mentoringId가 요청한mentoring.getId()와 일치하는지
assertThat(afterChatRoom.getMentoringId()).isEqualTo(mentoring.getId())로 함께 검증해 견고함을 높입니다.then 절 예시 변경:
- ChatRoom afterChatRoom = chatRoomRepositoryForTest - .findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()) - .orElseThrow(); - List<Long> participantIds = afterChatRoom.getChatParticipants().stream() - .map(ChatParticipant::getSiteUserId) - .toList(); - assertAll( - () -> assertThat(afterChatRoom.isGroup()).isFalse(), - () -> assertThat(participantIds).containsExactly(mentorUser1.getId(), menteeUser.getId()) - ); + await().atMost(Duration.ofSeconds(3)).untilAsserted(() -> { + ChatRoom afterChatRoom = chatRoomRepositoryForTest + .findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()) + .orElseThrow(); + List<Long> participantIds = afterChatRoom.getChatParticipants().stream() + .map(ChatParticipant::getSiteUserId) + .toList(); + assertAll( + () -> assertThat(afterChatRoom.isGroup()).isFalse(), + () -> assertThat(participantIds).containsExactly(mentorUser1.getId(), menteeUser.getId()), + () -> assertThat(afterChatRoom.getMentoringId()) + .isEqualTo(mentoring.getId()) + ); + });필요한 추가 import:
import static org.awaitility.Awaitility.await; import java.time.Duration;src/main/java/com/example/solidconnection/mentor/service/MentoringEventHandler.java (1)
19-20: 비동기 처리 설정 확인 완료 및 추가 조치 불필요 안내
- 선언 위치 확인
- src/main/java/com/example/solidconnection/scheduler/UpdateViewCountScheduler.java:20
- src/main/java/com/example/solidconnection/s3/service/FileUploadService.java:21
- src/main/java/com/example/solidconnection/community/post/service/UpdateViewCountService.java:15
- 결론
- @async 어노테이션은 정상적으로 비동기 처리되며, 별도의 @EnableAsync 추가 설정이 필요 없습니다.
수고 많으셨습니다!
src/main/java/com/example/solidconnection/chat/domain/ChatRoom.java (1)
23-28: 검증 완료: mentoring_id Nullable 컬럼 Unique 제약
검증 결과, mentoring_id 컬럼에 대한 unique 제약은 의도대로 동작합니다.
다음과 같이 요약합니다:
- 제약조건 동작 검증
- MySQL, PostgreSQL, Oracle 등 주요 DB에서 NULL끼리 중복을 허용하여 mentoring_id가 존재할 때만 unique가 적용됨을 확인했습니다.
- 마이그레이션 스크립트 점검
- V28__add_mentoring_id_to_chat_room.sql에서 mentoring_id 컬럼 추가와 uk_chat_room_mentoring_id 제약 정의가 정확히 반영되었습니다.
- 권장사항
- 다른 DB 환경에서도 동일 동작을 보장하도록 테스트 케이스를 추가하는 것을 추천합니다.
| """) | ||
| long countUnreadMessages(@Param("chatRoomId") long chatRoomId, @Param("userId") long userId); | ||
|
|
||
| Optional<ChatRoom> findByMentoringId(long mentoringId); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
nullable 컬럼과 메서드 시그니처 타입 정합성 맞추기 제안(Long 권장)
아래 변경을 권장드립니다.
1) mentoringId는 DB/엔티티에서 nullable입니다. 파라미터를 원시형 long으로 고정하면 의미상 불일치가 생깁니다. Long으로 변경해 시그니처 정합성을 맞추는 편이 명확합니다.
2) 동시성(중복 생성) 방어. 서비스 레벨에서 find → save 사이 레이스가 발생할 수 있습니다. UNIQUE 제약으로 2차 방어는 되지만, DataIntegrityViolationException을 캐치해 재조회하는(멱등 처리) 로직을 함께 두면 운영 안정성이 높아집니다.
제안 diff:
- Optional<ChatRoom> findByMentoringId(long mentoringId);
+ Optional<ChatRoom> findByMentoringId(Long mentoringId);추가 팁(선택):
- 생성 경로에서 이 메서드로 선조회할 때, 필요 시 @lock(PESSIMISTIC_READ/WRITE) 활용을 검토해 보세요. 단, 락 범위/시간에 유의해야 합니다.
🏁 Script executed:
#!/bin/bash
# Find the ChatRoom entity class file
rg "class ChatRoom" -n
# Search for mentoringId field declaration and its annotations
rg "mentoringId" -nLength of output: 5399
🏁 Script executed:
#!/bin/bash
# mentoringId 필드와 컬럼 정의(nullable 여부) 확인
rg -n -C3 "mentoringId" src/main/java/com/example/solidconnection/chat/domain/ChatRoom.javaLength of output: 510
검토 사항: 메서드 시그니처와 동시성 방어 개선 제안
-
메서드 시그니처 정합성
- 엔티티의
mentoringId필드가Long(nullable) 타입이므로, 리포지토리 인터페이스의 파라미터도long대신Long으로 변경하는 편이 일관성이 높습니다. - 제안 변경:
- Optional<ChatRoom> findByMentoringId(long mentoringId); + Optional<ChatRoom> findByMentoringId(Long mentoringId);
- 엔티티의
-
동시성(중복 생성) 방어 로직 검토
- 서비스 레벨에서
find → save사이에 레이스가 발생할 수 있으므로, 2차 방어로 데이터베이스의 UNIQUE 제약 위반 시DataIntegrityViolationException을 캐치해 재조회하는 멱등 처리 로직을 추가하면 안정성이 높아집니다. - 참고: 이미 UNIQUE 제약이 설정돼 있다면, 삽입 예외 발생 시 재시도 패턴을 적용해보세요.
- 서비스 레벨에서
-
선택 팁: @lock 활용
- 채팅방 생성 전 선조회 시
@Lock(PESSIMISTIC_READ/WRITE)를 사용해 락을 걸면, 동시성 충돌 가능성을 더 낮출 수 있습니다. 락 범위와 소요 시간을 꼭 고려하세요.
- 채팅방 생성 전 선조회 시
📝 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.
| Optional<ChatRoom> findByMentoringId(long mentoringId); | |
| Optional<ChatRoom> findByMentoringId(Long mentoringId); |
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/chat/repository/ChatRoomRepository.java
at line 38, change the method parameter type from primitive long to Long to
match the nullable mentoringId field type in the entity for consistency.
Additionally, enhance concurrency safety by implementing a try-catch block
around the save operation in the service layer to catch
DataIntegrityViolationException caused by unique constraint violations, and upon
catching, re-query the database to return the existing record, ensuring
idempotent behavior. Optionally, consider adding @Lock annotations with
PESSIMISTIC_READ or PESSIMISTIC_WRITE on the find method to reduce race
conditions, but evaluate the impact on performance and lock duration before
applying.
| public void createMentoringChatRoom(Long mentoringId, Long mentorId, Long menteeId) { | ||
| Optional<ChatRoom> existingChatRoom = chatRoomRepository.findByMentoringId(mentoringId); | ||
| if (existingChatRoom.isPresent()) { | ||
| return; | ||
| } | ||
|
|
||
| ChatRoom chatRoom = new ChatRoom(mentoringId, false); | ||
| chatRoom = chatRoomRepository.save(chatRoom); | ||
| ChatParticipant mentorParticipant = new ChatParticipant(mentorId, chatRoom); | ||
| ChatParticipant menteeParticipant = new ChatParticipant(menteeId, chatRoom); | ||
| chatParticipantRepository.saveAll(List.of(mentorParticipant, menteeParticipant)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
유니크 제약 경쟁 조건을 예외 캐치로 멱등화하세요.
- 동시 이벤트로 두 트랜잭션이 모두 미존재로 판단 후 저장을 시도할 수 있습니다.
- DB 유니크 제약으로 한 쪽이 DataIntegrityViolationException 을 던질 수 있습니다.
- 예외를 캐치하여 조용히 리턴하면 멱등성과 운영 안정성이 향상됩니다.
public void createMentoringChatRoom(Long mentoringId, Long mentorId, Long menteeId) {
+ if (mentoringId == null) {
+ // 멘토링 기반 생성에서는 null이 될 수 없도록 방어합니다.
+ return;
+ }
Optional<ChatRoom> existingChatRoom = chatRoomRepository.findByMentoringId(mentoringId);
if (existingChatRoom.isPresent()) {
return;
}
- ChatRoom chatRoom = new ChatRoom(mentoringId, false);
- chatRoom = chatRoomRepository.save(chatRoom);
+ ChatRoom chatRoom = new ChatRoom(mentoringId, false);
+ try {
+ chatRoom = chatRoomRepository.save(chatRoom);
+ } catch (org.springframework.dao.DataIntegrityViolationException e) {
+ // 다른 트랜잭션이 선점하여 생성한 케이스로 간주하고 멱등하게 종료합니다.
+ return;
+ }
ChatParticipant mentorParticipant = new ChatParticipant(mentorId, chatRoom);
ChatParticipant menteeParticipant = new ChatParticipant(menteeId, chatRoom);
chatParticipantRepository.saveAll(List.of(mentorParticipant, menteeParticipant));
}📝 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 void createMentoringChatRoom(Long mentoringId, Long mentorId, Long menteeId) { | |
| Optional<ChatRoom> existingChatRoom = chatRoomRepository.findByMentoringId(mentoringId); | |
| if (existingChatRoom.isPresent()) { | |
| return; | |
| } | |
| ChatRoom chatRoom = new ChatRoom(mentoringId, false); | |
| chatRoom = chatRoomRepository.save(chatRoom); | |
| ChatParticipant mentorParticipant = new ChatParticipant(mentorId, chatRoom); | |
| ChatParticipant menteeParticipant = new ChatParticipant(menteeId, chatRoom); | |
| chatParticipantRepository.saveAll(List.of(mentorParticipant, menteeParticipant)); | |
| } | |
| public void createMentoringChatRoom(Long mentoringId, Long mentorId, Long menteeId) { | |
| if (mentoringId == null) { | |
| // 멘토링 기반 생성에서는 null이 될 수 없도록 방어합니다. | |
| return; | |
| } | |
| Optional<ChatRoom> existingChatRoom = chatRoomRepository.findByMentoringId(mentoringId); | |
| if (existingChatRoom.isPresent()) { | |
| return; | |
| } | |
| ChatRoom chatRoom = new ChatRoom(mentoringId, false); | |
| try { | |
| chatRoom = chatRoomRepository.save(chatRoom); | |
| } catch (org.springframework.dao.DataIntegrityViolationException e) { | |
| // 다른 트랜잭션이 선점하여 생성한 케이스로 간주하고 멱등하게 종료합니다. | |
| return; | |
| } | |
| ChatParticipant mentorParticipant = new ChatParticipant(mentorId, chatRoom); | |
| ChatParticipant menteeParticipant = new ChatParticipant(menteeId, chatRoom); | |
| chatParticipantRepository.saveAll(List.of(mentorParticipant, menteeParticipant)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/chat/service/ChatService.java
around lines 129 to 140, the createMentoringChatRoom method can face a race
condition where concurrent transactions both find no existing chat room and
attempt to save, causing a DataIntegrityViolationException due to unique
constraints. To fix this, wrap the save operation in a try-catch block that
catches DataIntegrityViolationException and silently returns to ensure
idempotency and improve operational stability.
There was a problem hiding this comment.
이전에 설정한 유니크 키로 race condition은 막을 수 있으니, 이에 대한 예외 처리만 추가하면 좋을 것 같습니다 !
ex) 이미 생성된 채팅방입니다.
There was a problem hiding this comment.
@whqtker 네, 맞습니다! 유니크 키 제약으로 데이터 무결성은 보장되므로, 동시 요청 시 발생할 수 있는 DataIntegrityViolationException을 적절히 처리하면 됩니다.
이벤트 핸들러에서 호출되는 메서드이므로, 예외를 조용히 처리하고 로깅하는 방식이 적합할 것 같습니다:
@Transactional
public void createMentoringChatRoom(Long mentoringId, Long mentorId, Long menteeId) {
Optional<ChatRoom> existingChatRoom = chatRoomRepository.findByMentoringId(mentoringId);
if (existingChatRoom.isPresent()) {
log.debug("멘토링 ID {}에 대한 채팅방이 이미 존재합니다.", mentoringId);
return;
}
ChatRoom chatRoom = new ChatRoom(mentoringId, false);
try {
chatRoom = chatRoomRepository.save(chatRoom);
ChatParticipant mentorParticipant = new ChatParticipant(mentorId, chatRoom);
ChatParticipant menteeParticipant = new ChatParticipant(menteeId, chatRoom);
chatParticipantRepository.saveAll(List.of(mentorParticipant, menteeParticipant));
} catch (DataIntegrityViolationException e) {
// 동시 요청으로 이미 다른 트랜잭션에서 생성된 경우
log.info("멘토링 ID {}에 대한 채팅방이 이미 생성되었습니다. (동시 요청)", mentoringId);
// 채팅방이 이미 존재하므로 정상 종료
return;
}
}필요한 import도 추가해야 합니다:
import org.springframework.dao.DataIntegrityViolationException;
import lombok.extern.slf4j.Slf4j;클래스 레벨에 @Slf4j 어노테이션도 추가하시면 됩니다.
이렇게 하면 동시 요청 시에도 안정적으로 처리되며, 로그를 통해 모니터링도 가능합니다.
There was a problem hiding this comment.
전 @ExceptionHandler(DataIntegrityViolationException.class) 여기서 예외를 잡아준다고 생각해서 따로 처리 안했는데
이건 무시하고 그냥 try catch로 잡아 던지는 걸 말씀하신 게 맞나요?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
아 그러네요 .. 저는 따로 커스텀 예외 처리 말한건데, DataIntegrityViolationException 처리가 이미 있으니 따로 안 해도 될 거 같네요 🤔
|
|
||
| private final MentoringRepository mentoringRepository; | ||
| private final MentorRepository mentorRepository; | ||
| private final ApplicationEventPublisher eventPublisher; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
이벤트 처리의 트랜잭션 격리 의도와 실제 동작 일치 확인(승인 트랜잭션 영향 차단)
요구사항은 “채팅방 생성 실패가 멘토링 승인에 영향 주지 않음”입니다. 아래를 점검해 주세요.
1) 리스너 동작 방식. ApplicationEventPublisher는 기본 동기 호출이며, 동기 리스너가 예외를 던지면 여기로 전파되어 승인 트랜잭션이 롤백될 수 있습니다. REQUIRES_NEW는 리스너 트랜잭션 분리엔 도움되지만, 예외 전파를 막아주진 않습니다.
2) 권장 패턴. MentoringEventHandler에 @TransactionalEventListener(phase = AFTER_COMMIT)로 후커밋 실행을 보장하고, 내부에서 @transactional(propagation = REQUIRES_NEW) + 예외 캐치/로깅으로 원 트랜잭션을 보호하세요. 비동기 분리를 원하면 @async를 추가하고 예외를 반드시 삼켜야 합니다.
3) 테스트와의 정합성. 현재 테스트가 승인 직후 곧바로 채팅방을 조회해 성공한다면, 리스너는 동기일 가능성이 높습니다(또는 동기-후커밋). 의도와 어긋날 수 있으니 리스너 설정을 확인해 주세요.
예시(핵심 아이디어):
- @TransactionalEventListener(phase = AFTER_COMMIT)
- @async (선택)
- try/catch로 예외 흡수 + 로깅
- ChatService는 @transactional(REQUIRES_NEW)
원하시면 MentoringEventHandler 예시 코드를 제안드리겠습니다.
Also applies to: 54-55
아래 스크립트를 실행해 이벤트 핸들러 설정을 검증해 주세요.
- 멘토링 이벤트 핸들러 클래스 존재 확인
- @eventlistener, @TransactionalEventListener, @async 어노테이션 유무 확인
- ChatService 호출 위치 확인
🏁 Script executed:
#!/bin/bash
rg "class MentoringEventHandler" -n src
rg "@EventListener" -n src
rg "@TransactionalEventListener" -n src
rg "@Async" -n src
rg "ChatService" -n srcLength of output: 1989
추가 검증을 위해 이벤트 핸들러 코드를 확인합니다.
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' src/main/java/com/example/solidconnection/mentor/service/MentoringEventHandler.javaLength of output: 1001
이벤트 핸들러 트랜잭션 격리 및 예외 처리 강화하기
- @TransactionalEventListener(phase = AFTER_COMMIT) 적용
승인 트랜잭션 커밋 이후에만 이벤트가 실행되도록 보장해 주세요. - 예외 캐치 및 로깅 추가
chatService.createMentoringChatRoom 호출을 try-catch로 감싸 예외 발생 시 로깅하고 승인 로직에 영향을 주지 않도록 보호하세요. - ChatService 새 트랜잭션 분리
createMentoringChatRoom 메서드에 @transactional(propagation = REQUIRES_NEW)를 붙여 별도 트랜잭션에서 안전하게 처리되게 해 주세요. - @async 유지(선택)
비동기 분리가 필요하다면 @async를 살려두되, 핸들러 내부에서 예외가 완전히 소화되도록 확인해 주세요.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/mentor/service/MentoringCommandService.java
at line 30, enhance the event handler by annotating it with
@TransactionalEventListener(phase = AFTER_COMMIT) to ensure it runs only after
the approval transaction commits. Wrap the call to
chatService.createMentoringChatRoom in a try-catch block to log exceptions
without affecting the approval logic. Modify createMentoringChatRoom in
ChatService to have @Transactional(propagation = REQUIRES_NEW) for separate
transaction handling. If @Async is used, keep it but ensure all exceptions
inside the handler are fully caught and handled.
src/main/java/com/example/solidconnection/mentor/service/MentoringEventHandler.java
Outdated
Show resolved
Hide resolved
| ALTER TABLE chat_room | ||
| ADD COLUMN mentoring_id BIGINT; | ||
|
|
||
| ALTER TABLE chat_room | ||
| ADD CONSTRAINT uk_chat_room_mentoring_id | ||
| UNIQUE (mentoring_id); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
유니크 제약의 NULL 처리와 운영 중 락 영향 점검 요청
아래 항목을 순서대로 확인해 주세요.
1) NULL-UNIQUE 동작 차이 검증. 대부분의 PostgreSQL/MySQL은 UNIQUE 컬럼에 대해 다중 NULL을 허용하지만, SQL Server는 기본적으로 NULL을 1개만 허용합니다. mentoring_id를 nullable로 유지하며 “값이 있을 때만 유일”을 원하신다면, DB 벤더가 SQL Server가 아닌지 반드시 확인해 주세요. 대안으로 벤더가 SQL Server/PostgreSQL인 경우 “mentoring_id IS NOT NULL” 조건의 필터드(부분) 유니크 인덱스가 더 안전합니다.
2) 운영 락 영향 최소화. ALTER TABLE … ADD CONSTRAINT는 대체로 테이블-레벨 락을 유발합니다. 대용량 테이블/피크 타임 배포라면 Postgres 기준 CREATE UNIQUE INDEX CONCURRENTLY(비트랜잭션) → 제약 전환(또는 인덱스만 사용) 전략을 고려해 주세요. Flyway에서 해당 마이그레이션을 트랜잭션 밖에서 실행해야 합니다.
3) 성능 관점 확인. UNIQUE 제약은 인덱스를 자동 생성하므로, mentoring_id 기반 조회 성능은 확보됩니다. 부분 유니크 인덱스를 쓰면 NULL 행이 많은 경우 인덱스 크기를 더 줄일 수 있습니다.
4) 데이터 무결성 검토. 향후 멘토링 엔티티와의 FK를 고려 중이라면(현재는 의도적으로 느슨한 결합이라면 생략 OK), 생성 시점 데이터 무결성 시나리오를 한 번 점검해 주세요.
5) 기존 데이터 영향. 기존 행에는 mentoring_id가 모두 NULL일 가능성이 크므로 현재 스크립트는 충돌 없이 적용될 확률이 높습니다. 운영 스키마에서 예외 케이스(이미 값이 있는 행)가 없는지 한번 더 확인해 주세요.
참고용(선택지, Postgres 기준):
- 부분 유니크 인덱스
CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS uk_chat_room_mentoring_id_notnull
ON chat_room (mentoring_id)
WHERE mentoring_id IS NOT NULL;
다음 스크립트로 DB 벤더 및 설정 단서를 빠르게 확인할 수 있어요. 결과 공유 부탁드려요.
🏁 Script executed:
#!/bin/bash
# DB 벤더 및 설정 단서 찾기
rg -n --no-heading -S "spring\.datasource\.url|spring\.r2dbc\.url|jdbc:|testcontainers|postgres|mysql|mariadb|sqlserver|h2" \
src/main/resources src/test/resources || true
# Testcontainers 사용 흔적
rg -n --no-heading -S "PostgreSQLContainer|MySQLContainer|MSSQLServerContainer|MariaDBContainer|jdbc:" \
src/test || trueLength of output: 1053
유니크 제약 및 운영 영향 체크 부탁드려요
-
NULL–UNIQUE 동작 확인
MySQL(InnoDB)은 UNIQUE 컬럼에 여러 개의 NULL을 허용하므로, 현재 스크립트로도 “값이 있을 때만 유일” 요구사항은 충족됩니다. -
운영 중 테이블 락 최소화
MySQL의ALTER TABLE … ADD CONSTRAINT는 전체 테이블 락을 유발합니다. 대용량 테이블 또는 피크 타임 배포라면
-ALTER TABLE … ALGORITHM=INPLACE옵션 사용
-pt-online-schema-change등의 온라인 DDL 도구 적용
전략을 고려해 주세요. -
인덱스 및 성능 검토
MySQL은 UNIQUE 제약 추가 시 자동으로 인덱스를 생성합니다. 조회 성능은 확보되지만, MySQL은 부분 유니크 인덱스를 지원하지 않아
널이 많은 경우 인덱스 크기 최적화 옵션은 제한적입니다. -
데이터 무결성 시나리오 점검
향후mentoring엔티티와 FK 연계를 계획 중이라면, 제약 생성 시점의 데이터 무결성(기존 값·동시성) 시나리오를 한 번 검토해 주세요. -
기존 데이터 영향 확인
대부분 기존 행의mentoring_id는 NULL 예상되므로 충돌 없이 적용 가능하지만, 혹시 NULL이 아닌 값이 있는 예외 케이스가 있는지 한번 더 살펴봐 주세요.
🤖 Prompt for AI Agents
In src/main/resources/db/migration/V28__add_mentoring_id_to_chat_room.sql lines
1 to 6, the unique constraint on mentoring_id allows multiple NULLs in MySQL,
which fits the requirement. To minimize table locking during deployment on large
or busy tables, modify the ALTER TABLE statements to use ALGORITHM=INPLACE or
apply an online schema change tool like pt-online-schema-change. Also, review
existing data for any non-NULL mentoring_id values to avoid conflicts and
consider future foreign key constraints and data integrity scenarios before
applying the migration.
src/test/java/com/example/solidconnection/chat/repository/ChatRoomRepositoryForTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/chat/repository/ChatRoomRepositoryForTest.java
Show resolved
Hide resolved
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
고생하셨습니다 ~! 구현해야 하는 내용 보니 스프링 이벤트와 트랜잭션에 엄청 신경쓰셨을 것 같습니다 ... 몇 개 코멘트 남겼습니다 !
src/main/java/com/example/solidconnection/chat/domain/ChatRoom.java
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V28__add_mentoring_id_to_chat_room.sql
Outdated
Show resolved
Hide resolved
| public void createMentoringChatRoom(Long mentoringId, Long mentorId, Long menteeId) { | ||
| Optional<ChatRoom> existingChatRoom = chatRoomRepository.findByMentoringId(mentoringId); | ||
| if (existingChatRoom.isPresent()) { | ||
| return; | ||
| } | ||
|
|
||
| ChatRoom chatRoom = new ChatRoom(mentoringId, false); | ||
| chatRoom = chatRoomRepository.save(chatRoom); | ||
| ChatParticipant mentorParticipant = new ChatParticipant(mentorId, chatRoom); | ||
| ChatParticipant menteeParticipant = new ChatParticipant(menteeId, chatRoom); | ||
| chatParticipantRepository.saveAll(List.of(mentorParticipant, menteeParticipant)); | ||
| } |
There was a problem hiding this comment.
이전에 설정한 유니크 키로 race condition은 막을 수 있으니, 이에 대한 예외 처리만 추가하면 좋을 것 같습니다 !
ex) 이미 생성된 채팅방입니다.
src/main/java/com/example/solidconnection/mentor/service/MentoringEventHandler.java
Outdated
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/chat/repository/ChatRoomRepositoryForTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/chat/repository/ChatRoomRepositoryForTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/main/resources/db/migration/V28__add_mentoring_id_to_chat_room.sql (1)
3-3: 2) NULL-UNIQUE 동작은 DB마다 달라요(요구사항 재확인 필요).
- mentoring_id가 NULL일 때 다중 허용 여부는 벤더별로 달라요.
- PostgreSQL/MySQL/MariaDB: UNIQUE 컬럼에 다중 NULL 허용.
- SQL Server: 기본값으로 NULL 1개만 허용 → “값이 있을 때만 유일”을 보장하려면 Filtered Unique Index가 필요.
- 우리 운영 DB를 확인해 주세요. SQL Server라면 필터드 인덱스로 바꿔야 합니다.
참고: 지난 코멘트와 동일 주제라 중복 태깅합니다.
🧹 Nitpick comments (3)
src/main/resources/db/migration/V28__add_mentoring_id_to_chat_room.sql (1)
2-2: 4) 명시적으로 NULL 허용을 표기해 가독성을 올려요.
- 대부분 벤더에서 기본이 NULL 허용이지만, DDL 컨벤션상 의도를 드러내면 좋습니다.
-ADD COLUMN mentoring_id BIGINT, +ADD COLUMN mentoring_id BIGINT NULL,src/main/java/com/example/solidconnection/mentor/service/MentoringEventHandler.java (2)
18-23: 2) 관측 가능성(로그)과 실패 처리 가시성을 조금만 보강해요.
- 운영 디버깅 편의를 위해 시작/성공/실패 로그를 남기면 좋아요.
- 슬림하게 @slf4j 추가 후, 멘토링/멘토/멘티 ID를 키로 남기는 걸 추천합니다.
@@ -import lombok.RequiredArgsConstructor; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; @@ -@RequiredArgsConstructor +@RequiredArgsConstructor +@Slf4j public class MentoringEventHandler { @@ - public void handleMentoringApproved(MentoringApprovedEvent event) { - chatService.createMentoringChatRoom(event.mentoringId(), event.mentorId(), event.menteeId()); - } + public void handleMentoringApproved(MentoringApprovedEvent event) { + log.info("[MentoringApprovedEvent] start: mentoringId={}, mentorId={}, menteeId={}", + event.mentoringId(), event.mentorId(), event.menteeId()); + chatService.createMentoringChatRoom(event.mentoringId(), event.mentorId(), event.menteeId()); + log.info("[MentoringApprovedEvent] success: mentoringId={}", event.mentoringId()); + }
18-21: 4) @async 실행기 명시를 고려해요(부하 제어/분리).
- 기본 실행기 사용 시, 다른 @async 작업과 리소스를 섞어 쓸 수 있어요.
- 트래픽이 몰리는 이벤트라면 전용 Executor로 분리하면 안전합니다.
예시:
- @Async + @Async("chatEventExecutor") @Transactional(propagation = Propagation.REQUIRES_NEW) @TransactionalEventListener그리고 설정에 전용 ThreadPoolTaskExecutor(beanName="chatEventExecutor")를 추가해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/example/solidconnection/chat/domain/ChatRoom.java(3 hunks)src/main/java/com/example/solidconnection/chat/repository/ChatRoomRepository.java(1 hunks)src/main/java/com/example/solidconnection/chat/service/ChatService.java(1 hunks)src/main/java/com/example/solidconnection/mentor/service/MentoringEventHandler.java(1 hunks)src/main/resources/db/migration/V28__add_mentoring_id_to_chat_room.sql(1 hunks)src/test/java/com/example/solidconnection/chat/repository/ChatRoomRepositoryForTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/example/solidconnection/chat/repository/ChatRoomRepository.java
- src/main/java/com/example/solidconnection/chat/domain/ChatRoom.java
- src/test/java/com/example/solidconnection/chat/repository/ChatRoomRepositoryForTest.java
- src/main/java/com/example/solidconnection/chat/service/ChatService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/main/resources/db/migration/V28__add_mentoring_id_to_chat_room.sql (1)
5-6: 마이그레이션 파일에 ‘6’ 문자는 없습니다검증 결과, 파일 끝에 숫자 ‘6’이 전혀 존재하지 않습니다.
- 파일 내용 확인
- 1~4라인에 SQL 문만 있고, 5번째 라인은 빈 줄로 끝납니다.
- 비가시 문자 확인
- 숨은 특수문자나 ‘6’ 문자가 전혀 발견되지 않았습니다.
따라서 해당 리뷰 코멘트는 잘못된 지적이므로 무시하시고 안전하게 머지하세요.
Likely an incorrect or invalid review comment.
src/main/java/com/example/solidconnection/mentor/service/MentoringEventHandler.java (1)
18-23: 1) AFTER_COMMIT + REQUIRES_NEW + @async 조합, 의도와 잘 맞습니다.
- 멘토링 승인 커밋 이후에만 실행되고, 신규 트랜잭션에서 격리되며, 비동기로 주 쓰레드를 방해하지 않아요.
- 팀 논의 내용(승인 실패 영향 차단)과 정확히 부합합니다. 굿입니다.
| @Async | ||
| @Transactional(propagation = Propagation.REQUIRES_NEW) | ||
| @TransactionalEventListener | ||
| public void handleMentoringApproved(MentoringApprovedEvent event) { | ||
| chatService.createMentoringChatRoom(event.mentoringId(), event.mentorId(), event.menteeId()); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
3) 멱등성/경쟁 상태 대비 여부 확인 부탁드립니다.
- createMentoringChatRoom 구현이 “존재하면 패스, 없으면 생성” 형태라면, 다중 이벤트/재시도/동시성에서 DB UQ 충돌로 DataIntegrityViolationException이 날 수 있어요.
- 최종 방어선은 DB UQ가 맞지만, 서비스 레벨에서 UQ 충돌은 정상 시나리오로 취급(로그 레벨 낮춤·무시)하는 처리가 있으면 운영 소음이 줄어듭니다.
- ChatService 쪽에서 UQ 위반을 캐치해 멱등 처리하는지 한번만 확인해 주세요.
아래 스크립트로 구현 확인 가능해요(예외 처리 유무, existsByMentoringId 사용 여부):
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[Search] createMentoringChatRoom 구현"
rg -n --no-heading "createMentoringChatRoom\\s*\\(" -A 40
echo
echo "[Search] 멘토링 채팅 UQ/예외 처리 흔적"
rg -n --no-heading "DataIntegrityViolationException|ConstraintViolationException|existsByMentoringId"Length of output: 4695
멱등성·경쟁 상태 보강 요청
- 멱등성 체크 현황
ChatService.createMentoringChatRoom메소드에서chatRoomRepository.existsByMentoringId(mentoringId)로 이미 생성된 채팅방은 재생성하지 않습니다.
- 잠재적 경쟁 조건
- 동시 다발적인 이벤트 처리 시 두 스레드가 거의 동시에
existsBy검사를 통과하고save단계에서 DB의 고유 제약(UQ)을 위반해DataIntegrityViolationException이 발생할 수 있습니다.
- 동시 다발적인 이벤트 처리 시 두 스레드가 거의 동시에
- 서비스 레벨 방어선 추가
createMentoringChatRoom내부에서chatRoomRepository.save(...)와chatParticipantRepository.saveAll(...)호출을형태로 감싸고, UQ 위반 시 DEBUG 레벨로 로깅 후 무시 처리해주세요.try { // 기존 save 로직 } catch (DataIntegrityViolationException ex) { log.debug("중복 채팅방 생성 시도 무시: mentoringId={}", mentoringId, ex); }
- 참고로
CustomExceptionHandler는 주로 컨트롤러 레이어의 예외를 처리하므로, 비동기 이벤트 핸들러 내 예외는 이 핸들러에서 잡히지 않습니다.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/mentor/service/MentoringEventHandler.java
around lines 18 to 23, add service-level defense against race conditions by
catching DataIntegrityViolationException around the chatRoomRepository.save(...)
and chatParticipantRepository.saveAll(...) calls invoked by
ChatService.createMentoringChatRoom: wrap the existing save logic in a try/catch
that catches DataIntegrityViolationException, logs a DEBUG message like "중복 채팅방
생성 시도 무시: mentoringId={}" with the mentoringId and exception, and then silently
return/ignore the exception so concurrent duplicate-creation attempts do not
crash the async event handler.
| ALTER TABLE chat_room | ||
| ADD COLUMN mentoring_id BIGINT, | ||
| ADD CONSTRAINT uk_chat_room_mentoring_id UNIQUE (mentoring_id), | ||
| ADD CONSTRAINT fk_chat_room_mentoring_id FOREIGN KEY (mentoring_id) REFERENCES mentoring(id); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
1) 운영 락/가용성: 온라인 DDL 전략으로 분리해 주세요.
-
한 문장으로 ALTER TABLE에 컬럼/UNIQUE/FK를 몰아 넣으면 대용량에서 쓰기 락이 길어질 수 있어요.
-
배포 안전성을 위해 벤더별 온라인 DDL로 나누는 걸 권장해요.
- PostgreSQL 권장 예시
- 부분 유니크 인덱스 + CONCURRENTLY + FK NOT VALID → VALIDATE 순서로 록 최소화.
- 이 경우 해당 마이그레이션은 트랜잭션 밖에서 실행되어야 해요(executeInTransaction=false 분리, 또는 별도 마이그레이션 분할).
- PostgreSQL 권장 예시
-ALTER TABLE chat_room
- ADD COLUMN mentoring_id BIGINT,
- ADD CONSTRAINT uk_chat_room_mentoring_id UNIQUE (mentoring_id),
- ADD CONSTRAINT fk_chat_room_mentoring_id FOREIGN KEY (mentoring_id) REFERENCES mentoring(id);
+-- 1) 컬럼 추가
+ALTER TABLE chat_room ADD COLUMN mentoring_id BIGINT;
+
+-- 2) NULL 제외 부분 유니크 인덱스(온라인)
+CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS uk_chat_room_mentoring_id_notnull
+ ON chat_room (mentoring_id) WHERE mentoring_id IS NOT NULL;
+
+-- 3) FK는 즉시 검증하지 않고 등록(삭제 정책은 SET NULL 예시)
+ALTER TABLE chat_room
+ ADD CONSTRAINT fk_chat_room_mentoring_id
+ FOREIGN KEY (mentoring_id) REFERENCES mentoring(id) ON DELETE SET NULL
+ NOT VALID;
+
+-- 4) 트래픽 한가한 시점에 검증
+ALTER TABLE chat_room VALIDATE CONSTRAINT fk_chat_room_mentoring_id;2. MySQL(InnoDB) 권장 예시
- 인덱스/컬럼 추가는 ALGORITHM=INPLACE, LOCK=NONE(가능 범위) 고려.
- FK 추가는 내부적으로 테이블 잠금이 필요할 수 있어, 트래픽 한가한 시간대 분리 적용 또는 pt-online-schema-change 고려.
-ALTER TABLE chat_room
- ADD COLUMN mentoring_id BIGINT,
- ADD CONSTRAINT uk_chat_room_mentoring_id UNIQUE (mentoring_id),
- ADD CONSTRAINT fk_chat_room_mentoring_id FOREIGN KEY (mentoring_id) REFERENCES mentoring(id);
+ALTER TABLE chat_room
+ ADD COLUMN mentoring_id BIGINT NULL,
+ ALGORITHM=INPLACE, LOCK=NONE;
+
+CREATE UNIQUE INDEX uk_chat_room_mentoring_id ON chat_room (mentoring_id)
+ ALGORITHM=INPLACE, LOCK=NONE;
+
+ALTER TABLE chat_room
+ ADD CONSTRAINT fk_chat_room_mentoring_id
+ FOREIGN KEY (mentoring_id) REFERENCES mentoring(id) ON DELETE SET NULL;- 필요하시면 Flyway 마이그레이션을 벤더별로 분기(프로필/환경)하거나, 컨커런트 인덱스가 필요한 경우 해당 스텝만 별도(비트랜잭션) 마이그레이션으로 분리하도록 초안 드릴게요.
운영 가용성 최적화를 위한 온라인 DDL 단계 분리 제안
-
PostgreSQL 온라인 DDL 단계 분리
- mentoring_id 컬럼을 ALTER TABLE로 먼저 추가해주세요.
- CONCURRENTLY 옵션으로 NULL 제외 부분 유니크 인덱스를 생성해주세요.
- NOT VALID 옵션으로 ON DELETE SET NULL 정책의 외래키 제약을 추가해주세요.
- 이후 트래픽 여유 시점에 VALIDATE CONSTRAINT로 제약을 검증해주세요.
-
MySQL InnoDB 온라인 DDL 단계 분리
- ALGORITHM=INPLACE, LOCK=NONE 옵션으로 mentoring_id 컬럼을 추가해주세요.
- 동일한 옵션으로 유니크 인덱스를 생성해주세요.
- ON DELETE SET NULL 정책으로 외래키 제약을 추가하고, 필요 시 트래픽 여유 시점에 검증을 고려해주세요.
-
Flyway 마이그레이션 분리
- 벤더별 또는 비트랜잭션 단위로 마이그레이션을 분리하여 executeInTransaction=false 설정을 적용해주세요.
🤖 Prompt for AI Agents
In src/main/resources/db/migration/V28__add_mentoring_id_to_chat_room.sql around
lines 1 to 4, the current migration adds the mentoring_id column, unique
constraint and foreign key in a single blocking ALTER which risks downtime;
change to vendor-safe, staged DDL: for PostgreSQL, first ALTER TABLE ADD COLUMN
mentoring_id BIGINT; then create the unique index on mentoring_id WHERE
mentoring_id IS NOT NULL USING CONCURRENTLY; then add the FK with NOT VALID and
ON DELETE SET NULL, and validate it later with ALTER TABLE VALIDATE CONSTRAINT
when traffic allows; for MySQL/InnoDB, perform ALTER TABLE ... ADD COLUMN
mentoring_id BIGINT ALGORITHM=INPLACE, LOCK=NONE, then CREATE UNIQUE INDEX with
ALGORITHM=INPLACE, LOCK=NONE, then add the FK with ON DELETE SET NULL (consider
validating or checking afterward); finally split Flyway migrations by vendor or
into non-transactional steps and set executeInTransaction=false for the
non-transactional/CONCURRENTLY steps so they run safely outside a transaction.
| ALTER TABLE chat_room | ||
| ADD COLUMN mentoring_id BIGINT, | ||
| ADD CONSTRAINT uk_chat_room_mentoring_id UNIQUE (mentoring_id), | ||
| ADD CONSTRAINT fk_chat_room_mentoring_id FOREIGN KEY (mentoring_id) REFERENCES mentoring(id); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
3) FK 삭제 정책 명시를 권장(도메인 의도에 맞게).
- 현재는 기본 RESTRICT/NO ACTION로, mentoring 삭제 시 chat_room 삭제/유지 정책이 불명확해요.
- 칼럼이 NULL 허용이므로, “멘토링 삭제 시 방은 남기되 연결만 끊기”라면 ON DELETE SET NULL이 자연스럽습니다.
- “멘토링과 생명주기를 같이 한다”면 ON DELETE CASCADE가 맞고요.
예시(SET NULL):
-ADD CONSTRAINT fk_chat_room_mentoring_id FOREIGN KEY (mentoring_id) REFERENCES mentoring(id);
+ADD CONSTRAINT fk_chat_room_mentoring_id
+ FOREIGN KEY (mentoring_id) REFERENCES mentoring(id) ON DELETE SET NULL;외래키 삭제 정책 명시 요청
점검 위치:
- 파일: src/main/resources/db/migration/V28__add_mentoring_id_to_chat_room.sql
- 라인: 4
워크스루:
- 삭제 정책 추가
기본 RESTRICT/NO ACTION으로 mentoring 삭제 시 chat_room 처리 방식이 불명확합니다. - 도메인 의도에 맞는 정책 선택
- 채팅방은 그대로 유지하고 연결만 끊으려면 ON DELETE SET NULL을 사용하세요.
- mentoring과 생명주기를 함께 관리하려면 ON DELETE CASCADE를 사용하세요.
- 예시 diff
-ADD CONSTRAINT fk_chat_room_mentoring_id FOREIGN KEY (mentoring_id) REFERENCES mentoring(id);
+ADD CONSTRAINT fk_chat_room_mentoring_id
+ FOREIGN KEY (mentoring_id) REFERENCES mentoring(id) ON DELETE SET 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.
| ADD CONSTRAINT fk_chat_room_mentoring_id FOREIGN KEY (mentoring_id) REFERENCES mentoring(id); | |
| ADD CONSTRAINT fk_chat_room_mentoring_id | |
| FOREIGN KEY (mentoring_id) REFERENCES mentoring(id) ON DELETE SET NULL; |
🤖 Prompt for AI Agents
In src/main/resources/db/migration/V28__add_mentoring_id_to_chat_room.sql around
line 4, the FOREIGN KEY constraint is added without an explicit ON DELETE policy
which leaves behavior ambiguous; update the constraint to include a
domain-appropriate deletion policy (either ON DELETE SET NULL if chat_room
should remain and just unlink mentoring, or ON DELETE CASCADE if chat_room
should be removed when mentoring is removed), and if you choose SET NULL ensure
the mentoring_id column is nullable (alter column definition in this migration
if necessary); replace the current ADD CONSTRAINT line with the chosen FOREIGN
KEY clause including ON DELETE and apply corresponding column nullability
changes.
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
변경사항 확인했습니다 ! 어려운 구현이었을 텐데, 정말 고생 많으셨습니다 !!
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java (3)
11-13: 2) 테스트 도메인/리포지토리 import 구성 적절채팅방/참가자 도메인과 테스트 전용 리포지토리 의존이 명확해 가독성이 좋습니다.
29-31: 3) Duration/List/Optional import 적절비동기 검사와 컬렉션 변환에 필요한 최소 import로 구성되어 있습니다.
51-53: 4) 테스트에서 전용 리포지토리 주입 합리적프로덕션 로직을 침범하지 않고 조회 전용으로 상태를 검증하는 방향이 타당합니다.
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
| import static org.junit.jupiter.api.Assertions.assertAll; | ||
| import static org.testcontainers.shaded.org.awaitility.Awaitility.await; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
1) Testcontainers가 shade한 Awaitility 대신 공식 Awaitility 사용 권장
Shaded 패키지에 의존하면 업그레이드 시 경로 변경으로 테스트가 깨질 수 있습니다. 안정성을 위해 org.awaitility.Awaitility.await로 교체해 주세요.
적용 diff:
-import static org.testcontainers.shaded.org.awaitility.Awaitility.await;
+import static org.awaitility.Awaitility.await;직접 의존성도 추가해 주세요(Gradle 예시):
testImplementation "org.awaitility:awaitility:4.2.0"다음 스크립트로 shaded import 사용 여부와 직접 의존성 선언 여부를 확인할 수 있습니다.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[검색] shaded Awaitility import 사용 위치"
rg -n --hidden $'org\\.testcontainers\\.shaded\\.org\\.awaitility\\.Awaitility' || true
echo
echo "[검색] 직접 Awaitility import 사용 위치"
rg -n --hidden $'org\\.awaitility\\.Awaitility' || true
echo
echo "[검사] 빌드 스크립트 내 awaitility 의존성 선언 여부"
fd -a -t f -g "*build.gradle*" -g "*build.gradle.kts*" -g "pom.xml" | while read -r f; do
echo "==> $f"
rg -n 'awaitility' "$f" || true
doneLength of output: 963
다음 스크립트로 non-shaded import와 빌드 파일 내 Awaitility 의존성 선언 여부를 재검증해 주세요.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[검색] non-shaded Awaitility import 위치"
rg -n "import static org.awaitility.Awaitility.await" || true
echo
echo "[검사] build 파일 내 Awaitility 의존성 선언 여부"
rg -n "awaitility" -g "*.gradle*" -g "pom.xml" || trueLength of output: 308
Awaitility import 및 의존성 정비 필요
검증 결과, Testcontainers가 shade한 Awaitility import만 사용 중이며 공식 Awaitility import와 의존성 선언이 누락되어 있습니다.
안정적인 버전 업그레이드를 위해 아래 변경사항을 반영해 주세요:
- import 교체
- 변경 전:
import static org.testcontainers.shaded.org.awaitility.Awaitility.await; - 변경 후:
import static org.awaitility.Awaitility.await;
- 변경 전:
- 의존성 추가
- Gradle 예시:
testImplementation "org.awaitility:awaitility:4.2.0"
- Gradle 예시:
🤖 Prompt for AI Agents
In
src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java
around line 9, replace the Testcontainers-shaded Awaitility import with the
official Awaitility import (change import static
org.testcontainers.shaded.org.awaitility.Awaitility.await; to import static
org.awaitility.Awaitility.await;) and update your build configuration to add the
Awaitility test dependency (e.g., in Gradle add testImplementation
"org.awaitility:awaitility:4.2.0") so tests use the official library rather than
the shaded copy.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 현재 Awaitility import 상태 확인 ==="
echo "[1] shaded Awaitility import 사용 위치 검색"
rg -n "org\.testcontainers\.shaded\.org\.awaitility\.Awaitility" || echo "shaded import 없음"
echo
echo "[2] 공식 Awaitility import 사용 위치 검색"
rg -n "org\.awaitility\.Awaitility" || echo "공식 import 없음"
echo
echo "[3] 해당 테스트 파일의 현재 import 문 확인"
grep -n "awaitility\|Awaitility" src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java || echo "해당 파일에 Awaitility import 없음"
echo
echo "=== 빌드 설정에서 Awaitility 의존성 확인 ==="
echo "[4] 빌드 파일에서 awaitility 의존성 검색"
find . -name "*.gradle*" -o -name "pom.xml" | xargs grep -n "awaitility" || echo "빌드 파일에 awaitility 의존성 없음"
Length of output: 1106
@Gyuhyeok99 완벽하게 반영하셨네요! 🎉
1. Import 문 교체 완료
- ✅ shaded import 제거됨
- ✅ 공식 Awaitility import 적용:
import static org.awaitility.Awaitility.await;
2. 의존성 추가 완료
- ✅ build.gradle에
testImplementation 'org.awaitility:awaitility:4.2.0'추가
이제 Testcontainers의 shaded 버전에 의존하지 않아 향후 업그레이드 시에도 안정적으로 동작할 것입니다. 빠른 수정 감사합니다!
🐰✨
╭─────╮
│ ^_^ │ 수정 완료!
╰─────╯
╱ ╲
╱ ╲
🧬 Code Graph Analysis Results
-
File: src/test/java/com/example/solidconnection/mentor/service/MentoringCheckServiceTest.java
Line range: 27-171
Description:
"""
Docstring-style description:- Purpose: Tests for MentoringCheckService including mentor- and mentee-side mentoring checks.
- Parameters involved: MentoringCheckService, MentoringFixture, SiteUserFixture, MentorFixture.
- Return values: CheckedMentoringsResponse (for mentor checks) and related side effects on Mentoring entities.
- Exceptions / error handling: When an unauthorized mentor attempts to check mentoring, a CustomException with ErrorCode.UNAUTHORIZED_MENTORING is thrown.
- Key behavior:
- A mentor can mark multiple mentorings as checked; checkedAtByMentor is populated.
- A mentee can mark mentorings as checked; checkedAtByMentee is populated.
- Access by a different mentor results in an unauthorized error.
- Important implementation notes: Uses mentoringFixture to create test data, and asserts on mentoring status fields and exception messages.
"""
-
File: src/test/java/com/example/solidconnection/mentor/service/MentoringQueryServiceTest.java
Line range: 29-235
Description:
"""
Docstring-style description:- Purpose: Tests for MentoringQueryService to retrieve mentorings for mentors and mentees with various statuses.
- Parameters involved: MentoringQueryService, SiteUserFixture, MentorFixture, MentoringFixture, Pageable.
- Return values: SliceResponse with MentoringForMentorResponse or MentoringForMenteeResponse, containing mentoring IDs, partner names, status, and related info.
- Exceptions / error handling: Unauthorized access (e.g., attempting to fetch restricted mentoring lists) results in CustomException with UNAUTHORIZED_MENTORING.
- Key behavior:
- For mentors: fetch all statuses, verify partner info (nickname, university, country) and relation (isChecked).
- For mentees: enforce status-based filtering (APPROVED, PENDING, etc.), and ensure unauthorized fetches throw exceptions.
- Pagination/next page behavior is validated (via SliceResponse).
- Important implementation notes: Creates test data via MentoringFixture for various status combinations and asserts on IDs, nicknames, and status fields.
"""
-
File: src/test/java/com/example/solidconnection/mentor/service/MentorQueryServiceTest.java
Line range: 33-241
Description:
"""
Docstring-style description:- Purpose: Tests for MentorQueryService to verify mentor detail retrieval and related user-relationship info.
- Parameters involved: MentorQueryService, SiteUserFixture, MentorFixture, ChannelFixture, UniversityFixture.
- Return values: MentorDetailResponse including mentor ID, nickname, universityName, country, and channels; also checks whether the current user has applied for mentorship.
- Exceptions / error handling: If a non-existent mentor is requested, a CustomException with ErrorCode.MENTOR_NOT_FOUND is thrown.
- Key behavior:
- Fetch mentor details including associated channels and related mentor/mentee application status.
- Determine whether the requesting user has an existing mentor application to the mentor.
- Important implementation notes: Uses Channel and University fixtures to assemble expected details; validates both data correctness and error handling for missing mentors.
"""
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| @Test | ||
| void 멘토링_승인시_채팅방이_자동으로_생성된다() { | ||
| // given | ||
| Mentoring mentoring = mentoringFixture.대기중_멘토링(mentor1.getId(), menteeUser.getId()); | ||
| MentoringConfirmRequest request = new MentoringConfirmRequest(VerifyStatus.APPROVED); | ||
|
|
||
| Optional<ChatRoom> beforeChatRoom = chatRoomRepositoryForTest.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()); | ||
| assertThat(beforeChatRoom).isEmpty(); | ||
|
|
||
| // when | ||
| mentoringCommandService.confirmMentoring(mentorUser1.getId(), mentoring.getId(), request); | ||
|
|
||
| // then | ||
| ChatRoom afterChatRoom = await() | ||
| .atMost(Duration.ofSeconds(3)) | ||
| .pollInterval(Duration.ofMillis(100)) | ||
| .until(() -> chatRoomRepositoryForTest | ||
| .findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()), | ||
| Optional::isPresent) | ||
| .orElseThrow(); | ||
|
|
||
| List<Long> participantIds = afterChatRoom.getChatParticipants().stream() | ||
| .map(ChatParticipant::getSiteUserId) | ||
| .toList(); | ||
| assertAll( | ||
| () -> assertThat(afterChatRoom.isGroup()).isFalse(), | ||
| () -> assertThat(participantIds).containsExactly(mentorUser1.getId(), menteeUser.getId()) | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
5) 1:1 채팅방 생성 검증의 내구성 강화: 순서 비의존, 크기/연결관계 추가 검사, 타임아웃 여유
참가자 순서가 저장 시점/ORM 설정에 따라 비결정적일 수 있어 순서 비의존 검증이 안전합니다. 아울러 2인 채널임을 명시적으로 검증하고, 채팅방이 멘토링과 연결되었는지도 확인해 주세요. CI 환경 변동에 대비해 타임아웃을 조금 늘리는 것도 권장합니다.
권장 diff:
- ChatRoom afterChatRoom = await()
- .atMost(Duration.ofSeconds(3))
+ ChatRoom afterChatRoom = await()
+ .atMost(Duration.ofSeconds(5))
.pollInterval(Duration.ofMillis(100))
.until(() -> chatRoomRepositoryForTest
.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()),
Optional::isPresent)
.orElseThrow();
List<Long> participantIds = afterChatRoom.getChatParticipants().stream()
.map(ChatParticipant::getSiteUserId)
.toList();
assertAll(
() -> assertThat(afterChatRoom.isGroup()).isFalse(),
+ () -> assertThat(afterChatRoom.getMentoringId()).isEqualTo(mentoring.getId()),
+ () -> assertThat(afterChatRoom.getChatParticipants()).hasSize(2),
- () -> assertThat(participantIds).containsExactly(mentorUser1.getId(), menteeUser.getId())
+ () -> assertThat(participantIds).containsExactlyInAnyOrder(mentorUser1.getId(), menteeUser.getId())
);📝 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.
| @Test | |
| void 멘토링_승인시_채팅방이_자동으로_생성된다() { | |
| // given | |
| Mentoring mentoring = mentoringFixture.대기중_멘토링(mentor1.getId(), menteeUser.getId()); | |
| MentoringConfirmRequest request = new MentoringConfirmRequest(VerifyStatus.APPROVED); | |
| Optional<ChatRoom> beforeChatRoom = chatRoomRepositoryForTest.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()); | |
| assertThat(beforeChatRoom).isEmpty(); | |
| // when | |
| mentoringCommandService.confirmMentoring(mentorUser1.getId(), mentoring.getId(), request); | |
| // then | |
| ChatRoom afterChatRoom = await() | |
| .atMost(Duration.ofSeconds(3)) | |
| .pollInterval(Duration.ofMillis(100)) | |
| .until(() -> chatRoomRepositoryForTest | |
| .findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()), | |
| Optional::isPresent) | |
| .orElseThrow(); | |
| List<Long> participantIds = afterChatRoom.getChatParticipants().stream() | |
| .map(ChatParticipant::getSiteUserId) | |
| .toList(); | |
| assertAll( | |
| () -> assertThat(afterChatRoom.isGroup()).isFalse(), | |
| () -> assertThat(participantIds).containsExactly(mentorUser1.getId(), menteeUser.getId()) | |
| ); | |
| } | |
| @Test | |
| void 멘토링_승인시_채팅방이_자동으로_생성된다() { | |
| // given | |
| Mentoring mentoring = mentoringFixture.대기중_멘토링(mentor1.getId(), menteeUser.getId()); | |
| MentoringConfirmRequest request = new MentoringConfirmRequest(VerifyStatus.APPROVED); | |
| Optional<ChatRoom> beforeChatRoom = chatRoomRepositoryForTest.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()); | |
| assertThat(beforeChatRoom).isEmpty(); | |
| // when | |
| mentoringCommandService.confirmMentoring(mentorUser1.getId(), mentoring.getId(), request); | |
| // then | |
| ChatRoom afterChatRoom = await() | |
| .atMost(Duration.ofSeconds(5)) | |
| .pollInterval(Duration.ofMillis(100)) | |
| .until(() -> chatRoomRepositoryForTest | |
| .findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()), | |
| Optional::isPresent) | |
| .orElseThrow(); | |
| List<Long> participantIds = afterChatRoom.getChatParticipants().stream() | |
| .map(ChatParticipant::getSiteUserId) | |
| .toList(); | |
| assertAll( | |
| () -> assertThat(afterChatRoom.isGroup()).isFalse(), | |
| () -> assertThat(afterChatRoom.getMentoringId()).isEqualTo(mentoring.getId()), | |
| () -> assertThat(afterChatRoom.getChatParticipants()).hasSize(2), | |
| () -> assertThat(participantIds).containsExactlyInAnyOrder(mentorUser1.getId(), menteeUser.getId()) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java
around lines 128 to 157, make the 1:1 chat-room assertions order-independent and
stronger: increase the await timeout (e.g., a few more seconds), assert the
chatParticipants size is 2, assert the chat room is not a group, verify the room
is linked to the mentoring (compare mentoring.getId() to
afterChatRoom.getMentoringId() or the equivalent field), and check that the set
of participant IDs contains both mentorUser1.getId() and menteeUser.getId()
without relying on order (use containsAll or compare Sets).
There was a problem hiding this comment.
@Gyuhyeok99 좋습니다! 테스트의 내구성을 높이는 변경사항들을 반영해 주셔서 감사합니다. 이제 CI 환경에서도 더 안정적으로 동작할 것입니다. 🎉
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| @Test | ||
| void 멘토링_거절시_채팅방이_자동으로_생성되지_않는다() { | ||
| // given | ||
| Mentoring mentoring = mentoringFixture.대기중_멘토링(mentor1.getId(), menteeUser.getId()); | ||
| MentoringConfirmRequest request = new MentoringConfirmRequest(VerifyStatus.REJECTED); | ||
|
|
||
| Optional<ChatRoom> beforeChatRoom = chatRoomRepositoryForTest.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()); | ||
| assertThat(beforeChatRoom).isEmpty(); | ||
|
|
||
| // when | ||
| mentoringCommandService.confirmMentoring(mentorUser1.getId(), mentoring.getId(), request); | ||
|
|
||
| // then | ||
| Optional<ChatRoom> afterChatRoom = chatRoomRepositoryForTest.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()); | ||
| assertThat(afterChatRoom).isEmpty(); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
6) 거절 케이스에서도 비동기 부작용을 방어적으로 검증
즉시 검증은 이벤트 처리 지연 시 거짓 양성 위험이 있습니다. 짧은 윈도우 동안 생성되지 않음을 기다리며 확인해 주세요.
권장 diff:
- // then
- Optional<ChatRoom> afterChatRoom = chatRoomRepositoryForTest.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId());
- assertThat(afterChatRoom).isEmpty();
+ // then
+ await()
+ .pollInterval(Duration.ofMillis(100))
+ .during(Duration.ofSeconds(1))
+ .until(() -> chatRoomRepositoryForTest
+ .findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId())
+ .isEmpty());📝 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.
| @Test | |
| void 멘토링_거절시_채팅방이_자동으로_생성되지_않는다() { | |
| // given | |
| Mentoring mentoring = mentoringFixture.대기중_멘토링(mentor1.getId(), menteeUser.getId()); | |
| MentoringConfirmRequest request = new MentoringConfirmRequest(VerifyStatus.REJECTED); | |
| Optional<ChatRoom> beforeChatRoom = chatRoomRepositoryForTest.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()); | |
| assertThat(beforeChatRoom).isEmpty(); | |
| // when | |
| mentoringCommandService.confirmMentoring(mentorUser1.getId(), mentoring.getId(), request); | |
| // then | |
| Optional<ChatRoom> afterChatRoom = chatRoomRepositoryForTest.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()); | |
| assertThat(afterChatRoom).isEmpty(); | |
| } | |
| @Test | |
| void 멘토링_거절시_채팅방이_자동으로_생성되지_않는다() { | |
| // given | |
| Mentoring mentoring = mentoringFixture.대기중_멘토링(mentor1.getId(), menteeUser.getId()); | |
| MentoringConfirmRequest request = new MentoringConfirmRequest(VerifyStatus.REJECTED); | |
| Optional<ChatRoom> beforeChatRoom = chatRoomRepositoryForTest.findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()); | |
| assertThat(beforeChatRoom).isEmpty(); | |
| // when | |
| mentoringCommandService.confirmMentoring(mentorUser1.getId(), mentoring.getId(), request); | |
| // then | |
| await() | |
| .pollInterval(Duration.ofMillis(100)) | |
| .during(Duration.ofSeconds(1)) | |
| .until(() -> chatRoomRepositoryForTest | |
| .findOneOnOneChatRoomByParticipants(mentorUser1.getId(), menteeUser.getId()) | |
| .isEmpty()); | |
| } |
There was a problem hiding this comment.
@Gyuhyeok99 좋습니다! 비동기 이벤트 처리 지연으로 인한 거짓 양성을 방지하는 방어적 검증 로직을 적용해 주셔서 감사합니다. 이제 테스트가 더 안정적으로 거절 케이스에서 채팅방이 생성되지 않음을 확인할 수 있습니다.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
2 participants
관련 이슈
작업 내용
성혁님 리뷰 반영해서 채팅방 생성 기능 구현했습니다.
디스코드에서 얘기했던 대로 ChatRoom에 mentoringId랑 유니크키 추가했고,
mentoringId가 null이면 유니크키가 안 깨지니까 나중에 커뮤니티 다대다 채팅 들어와도 문제 없을 거 같아서 long 대신 Long으로 구현하였습니다.
이렇게 하니까 isGroup은 사실 필요 없어진 거 같긴 한데 그냥 무시(?)했습니다 ㅎㅎ
멘토링 성사될 때 이벤트 기반으로 채팅방이 생성되게 했고,
채팅방 개설에서 예외가 나더라도 멘토링 승인에는 영향을 없게 하려고 Propagation.REQUIRES_NEW로 설정했습니다.
혹시 예외 터질 때 retry 등의 기능이 필요할 수도 있긴 한데, 당분간은 문제 없을 거 같기도 하고... 복잡해지는 것 같아 뺐습니다.
지금 생각하기엔 관리자 api로 채팅방 강제로 만들어주는 것을 차라리 구현하는 게 나을 수도 있지 않을까란 생각이 듭니다!
특이 사항
리뷰 요구사항 (선택)
flyway 스크립트 제대로 작성했나요?