Release v0.2.0#100
Merged
Merged
Conversation
- Room 엔티티에 gender 컬럼 추가, 방 생성 시 방장 성별로 저장 - 방 목록 조회(FindRoomsUseCase)에서 요청자 성별과 일치하는 방만 반환 - QueryDSL 경로: room.gender.eq(gender) WHERE 조건 추가 - LATERAL 경로: AND r.gender = :gender 조건 추가 (LATERAL 실행 전 필터링) - 방 신청(ApplyRoomUseCase)에서 성별 불일치 시 GENDER_MISMATCH(ROOM016) 에러 반환 - 목록 필터링(Layer 1) + 신청 차단(Layer 2) 2중 방어 구조 - 방 생성(CreateRoomUseCase)에서 이미 방에 속한 사용자 사전 검증 추가 - 기존 DB constraint 오류 대신 ALREADY_JOINED_USER(ROOM013) 반환 - 인덱스 교체: idx_room_status_created → (room_status, gender, created_at, room_no) - 에러 코드 추가: GENDER_MISMATCH(ROOM016) - 관련 단위 테스트 전체 반영
…이블-gender-컬럼-추가-및-인덱스-확장 [FEAT] 성별 기반 방 목록 필터링 및 신청 차단
…eful-shutdown-적용
Walkthrough사용자 성별을 Room 엔티티·검색·신청·생성 흐름에 도입하고, 룸 삭제 시 연관 채팅 데이터 삭제와 WebSocket 알림 전파 흐름을 이벤트 기반으로 추가했습니다. WebSocket 인증/핸드셰이크, 체크리스트 매핑 구현, 여러 리포지토리의 벌크 삭제 및 인덱스·스키마 변경, 운영용 graceful shutdown 설정도 포함됩니다. ChangesGender-based Room Filtering & Basic Room Flows
Room Deletion + Chat Notification Event Flow
Chat WebSocket Notification & Sending
WebSocket Security / Principal Wiring
Checklist Mapper & Entity Behavior Changes
Infrastructure / Config / Misc
Sequence DiagramsequenceDiagram
actor Host
participant DeleteController as DeleteRoomController
participant DeleteUseCase as DeleteRoomUseCase
participant RoomService as RoomService
participant Repo as DB
participant ChatService as ChatRoomService
participant ChatMemberService as ChatRoomMemberService
participant ChatMessageService as ChatMessageService
participant EventPub as ApplicationEventPublisher
participant EventListener as RoomDeletedEventListener
participant Notifier as ChatWebSocketNotificationEventListener
participant WebSocketSend as ChatWebSocketSendService
Host->>DeleteController: DELETE /api/rooms/{roomNo}
DeleteController->>DeleteUseCase: execute(hostNo, roomNo)
DeleteUseCase->>RoomService: findByIdForUpdate(roomNo) (pessimistic lock)
RoomService->>Repo: SELECT ... FOR UPDATE
Repo-->>RoomService: Room (host, counts, etc.)
DeleteUseCase->>RoomService: roommateService.leaveRoom(...) (if any)
DeleteUseCase->>RoomService: roomService.flush()
DeleteUseCase->>Repo: DELETE RoomRequest/RoomRule/RoomLike (bulk)
DeleteUseCase->>EventPub: publishEvent(RoomDeletedEvent(roomNo))
EventPub->>EventListener: RoomDeletedEvent
EventListener->>ChatService: findAllByRoomNo(roomNo)
ChatService->>Repo: SELECT chat_rooms...
EventListener->>ChatMemberService: findAllByChatRoom(...)
EventListener->>ChatMessageService: deleteAllByChatRoom(...)
EventListener->>ChatMemberService: deleteAllByChatRoom(...)
EventListener->>ChatService: deleteByChatRoomNo(chatRoomNo)
EventListener->>EventPub: publishEvent(ChatWebSocketNotificationEvent(...))
EventPub->>Notifier: ChatWebSocketNotificationEvent
Notifier->>WebSocketSend: notifyUser(userNo, payload)
Notifier->>WebSocketSend: broadcast(chatRoomNo, payload)
WebSocketSend->>WebSocketSend: retry/recover on failure (`@Retryable/`@Recover)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
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/project/dorumdorum/domain/room/infra/repository/RoomRepositoryImpl.java (1)
208-262:⚠️ Potential issue | 🟡 Minor
findLikedRooms와findAppliedRooms에 성별 필터링 추가를 검토해 주세요.
findByCursor메서드에는room.gender.eq(gender)필터가 적용되어 있지만,findLikedRooms와findAppliedRooms에는 동일한 필터가 없습니다. 사용자의 성별이 변경되거나 데이터 마이그레이션이 발생한 경우, 이전에 즐겨찾기/지원했던 방 중에서 사용자와 성별이 다른 방이 목록에 노출될 수 있습니다.이 엔드포인트들도 같은 성별의 방만 보여주는 정책이라면,
UserService를 통해 사용자의 성별을 먼저 조회한 후Gender파라미터로 함께 전달하여findByCursor와 동일하게 필터링하는 것을 권장합니다. 반대로 과거 이력은 성별 관계없이 모두 보여주는 것이 의도라면 주석으로 해당 정책을 명시해 두면 좋을 것 같습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/project/dorumdorum/domain/room/infra/repository/RoomRepositoryImpl.java` around lines 208 - 262, findLikedRooms and findAppliedRooms currently lack the same gender filtering used in findByCursor (room.gender.eq(gender)), which can surface rooms with mismatched genders after user changes or migrations; update these methods (findLikedRooms, findAppliedRooms) to accept a Gender parameter (or fetch the user's gender via UserService before calling) and add the predicate room.gender.eq(gender) to their where clauses, or if the intended behavior is to show history regardless of gender, add a clarifying comment in both methods documenting that decision instead of adding the filter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.coderabbit.yml:
- Around line 11-12: The base_branches setting currently uses the broad pattern
base_branches: ".*" which enables automatic reviews on PRs targeting every
branch; confirm whether this is intentional or narrow the scope by replacing the
".*" pattern with explicit branch names or more specific patterns (e.g., "main",
"develop", "release/.*") in the base_branches entry to limit review scope and
resource usage; update the .coderabbit.yml base_branches key accordingly and
ensure any required branch patterns are listed explicitly.
In @.gitignore:
- Line 61: 현재 .gitignore의 "docs/" 항목은 전체 문서 디렉터리를 통째로 무시해 수동으로 관리하는 문서 파일까지 추적에서
빠지게 됩니다; .gitignore에서 "docs/" 항목을 찾아 전체 무시 대신 생성 산출물만 제외하도록 "/docs/build/",
"/docs/dist/" 또는 특정 패턴("docs/**/*.html", "docs/**/*.pdf" 등)으로 좁혀 주고, 필요하면 수동 문서를
유지하려면 특정 파일을 예외 처리하는 방식(e.g., "!docs/README.md")을 사용하여 수동으로 관리하는 문서가 Git에 남도록
수정하세요.
In
`@src/main/java/com/project/dorumdorum/domain/room/application/usecase/ApplyRoomUseCase.java`:
- Around line 38-43: Replace the enum equality check using .equals(...) with an
identity comparison (use !=) between applicant.getGender() and room.getGender()
in ApplyRoomUseCase (after obtaining User via userService.findById(...) and Room
via roomService.findById(...)); change the condition that throws
RestApiException(GENDER_MISMATCH) from if
(!applicant.getGender().equals(room.getGender())) to if (applicant.getGender()
!= room.getGender()) to improve readability and avoid potential NPEs when
comparing the Gender enum values.
In
`@src/main/java/com/project/dorumdorum/domain/room/application/usecase/CreateRoomUseCase.java`:
- Around line 38-43: The check-and-create sequence in CreateRoomUseCase
(existsByUserNo -> roommateService.create) has a race; make it atomic by either
acquiring a DB pessimistic lock on the user/roommate row before checking (e.g.,
add/findByUserNoForUpdate in the roommate/user repository and use that lock in
CreateRoomUseCase or inside roommateService.create) or by keeping the optimistic
flow but catching the DB uniqueness violation when persisting the roommate
(catch DataIntegrityViolationException/PersistenceException thrown by
roommateService.create or its repository.save/flush) and translate that specific
unique-constraint error into throwing RestApiException(ALREADY_JOINED_USER);
update CreateRoomUseCase to use the locking variant or wrap the
roommateService.create call in a try/catch that maps unique-constraint failures
to ALREADY_JOINED_USER.
In `@src/main/java/com/project/dorumdorum/domain/user/domain/entity/User.java`:
- Around line 54-56: Confirm and fix migration for the new non-null constraint
on User.gender: (1) scan production DB for rows where gender IS NULL and
list/count them; (2) if any, add and run a backfill migration script
(referencing the schema change in schema.sql that does ALTER TABLE users ALTER
COLUMN gender SET NOT NULL) to populate a safe default or calculated value for
those rows before applying NOT NULL; (3) ensure your DDL migration process
(since application.yml uses ddl-auto: validate) will run the ALTER TABLE via
your migration tool (e.g., Flyway/Liquibase) rather than relying on Hibernate,
and add the NOT NULL DDL there; (4) add defensive handling where
User.getGender() is used (e.g., ApplyRoomUseCase and other callers) to avoid
NPEs until the backfill/constraint is in place (null checks or Optional) and
remove those safeguards once migration is complete.
In `@src/main/resources/schema.sql`:
- Around line 19-28: The migration is incomplete: it adds NOT NULL and indexes
on room.gender before the column exists or is backfilled; fix by updating the
migration to (1) ALTER TABLE room ADD COLUMN gender TYPE (matching users.gender)
nullable, (2) backfill room.gender from users.gender using host user relation
(e.g. join on host_user_id) for existing rows, (3) verify no NULLs remain
(raise/rollback if any), (4) ALTER TABLE room ALTER COLUMN gender SET NOT NULL,
and only then (5) create the indexes idx_room_status_gender_created and
idx_room_status_gender_remaining_created; also ensure users.gender NULLs are
handled (update or validate) before ALTER COLUMN users.gender SET NOT NULL.
In
`@src/test/java/com/project/dorumdorum/domain/room/unit/service/RoomServiceTest.java`:
- Around line 40-49: The test create_SavesRoom currently only asserts the
repository return value; update it to verify the actual Room passed into
roomRepository.save has gender set to Gender.MALE by capturing or matching the
argument from service.create("u1", Gender.MALE, request); use an ArgumentCaptor<
Room > or verify(roomRepository).save(argThat(r -> r.getGender() ==
Gender.MALE)) to assert the saved entity's gender, referencing
RoomService.create, RoomCreateRequest, roomRepository.save, Room and
Gender.MALE.
- Around line 72-79: The test currently stubs roomRepository.findByCursor using
any(Gender.class) which allows incorrect genders to pass; change the stubbing to
assert the exact gender (Gender.MALE) is passed by replacing any(Gender.class)
with an equality matcher for Gender.MALE (e.g., eq(Gender.MALE)) in the
when(...) for roomRepository.findByCursor, and if present add/adjust a
verify(...) on roomRepository.findByCursor to explicitly check the Gender.MALE
argument; this ensures service.searchByCursor(Gender.MALE, request, ...) is
actually forwarding the correct gender.
In
`@src/test/java/com/project/dorumdorum/domain/room/unit/usecase/ApplyRoomUseCaseTest.java`:
- Around line 100-104: The test only asserts the exception type from
useCase.execute but must also assert the specific error code for gender
mismatch; update the assertion that invokes useCase.execute(userNo, roomNo, new
JoinRoomRequest("intro", null)) to not only isInstanceOf(RestApiException.class)
but also verify the exception carries the GENDER_MISMATCH/ROOM016 error (e.g.,
inspect the RestApiException's error code or code getter and assert it equals
GENDER_MISMATCH or "ROOM016"), leaving the existing verify(...) calls unchanged.
In
`@src/test/java/com/project/dorumdorum/domain/room/unit/usecase/CreateRoomUseCaseTest.java`:
- Around line 50-53: The test CreateRoomUseCaseTest currently only asserts any
RestApiException from useCase.execute(userNo, request); change it to assert that
the thrown RestApiException contains the specific error state
ALREADY_JOINED_USER (e.g., by checking the exception's status/errorCode field or
using an assertion like hasFieldOrPropertyWithValue) and keep the existing
verify(roomService, never()).create(...) check; target the RestApiException
instance thrown by useCase.execute to ensure its error state equals
ALREADY_JOINED_USER.
---
Outside diff comments:
In
`@src/main/java/com/project/dorumdorum/domain/room/infra/repository/RoomRepositoryImpl.java`:
- Around line 208-262: findLikedRooms and findAppliedRooms currently lack the
same gender filtering used in findByCursor (room.gender.eq(gender)), which can
surface rooms with mismatched genders after user changes or migrations; update
these methods (findLikedRooms, findAppliedRooms) to accept a Gender parameter
(or fetch the user's gender via UserService before calling) and add the
predicate room.gender.eq(gender) to their where clauses, or if the intended
behavior is to show history regardless of gender, add a clarifying comment in
both methods documenting that decision instead of adding the filter.
🪄 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.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c0642854-b854-458f-b115-60ea397de761
📒 Files selected for processing (25)
.coderabbit.yml.gitignoreDockerfiledocker-compose.ymlsrc/main/java/com/project/dorumdorum/domain/room/application/usecase/ApplyRoomUseCase.javasrc/main/java/com/project/dorumdorum/domain/room/application/usecase/CreateRoomUseCase.javasrc/main/java/com/project/dorumdorum/domain/room/application/usecase/FindRoomsUseCase.javasrc/main/java/com/project/dorumdorum/domain/room/domain/entity/Room.javasrc/main/java/com/project/dorumdorum/domain/room/domain/repository/RoomQueryRepository.javasrc/main/java/com/project/dorumdorum/domain/room/domain/service/RoomService.javasrc/main/java/com/project/dorumdorum/domain/room/infra/repository/RoomRepositoryImpl.javasrc/main/java/com/project/dorumdorum/domain/room/ui/FindRoomsController.javasrc/main/java/com/project/dorumdorum/domain/room/ui/spec/FindRoomsApiSpec.javasrc/main/java/com/project/dorumdorum/domain/user/domain/entity/User.javasrc/main/java/com/project/dorumdorum/global/alert/ApplicationLifecycleAlertListener.javasrc/main/java/com/project/dorumdorum/global/exception/code/status/RoomErrorStatus.javasrc/main/resources/application-prod.ymlsrc/main/resources/schema.sqlsrc/test/java/com/project/dorumdorum/domain/chat/integration/ChatTransactionAtomicityIntegrationTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/infra/repository/RoomRepositoryImplTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/service/RoomServiceTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/ui/FindRoomsControllerTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/usecase/ApplyRoomUseCaseTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/usecase/CreateRoomUseCaseTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/usecase/FindRoomsUseCaseTest.java
Comment on lines
+11
to
+12
| base_branches: | ||
| - ".*" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
모든 base 브랜치에 대한 자동 리뷰 활성화를 확인해주세요.
base_branches에 ".*" 패턴을 설정하면 모든 브랜치를 대상으로 하는 PR에 대해 자동 리뷰가 실행됩니다. 문법적으로는 올바르지만, 리뷰 범위가 매우 넓어질 수 있습니다.
고려사항:
- feature 브랜치, experimental 브랜치 등 모든 브랜치를 타겟으로 하는 PR에서 리뷰가 트리거됩니다
- 리뷰 빈도와 리소스 사용량이 크게 증가할 수 있습니다
- 특정 브랜치 패턴만 필요한 경우 더 구체적인 패턴 사용을 권장합니다
대안 예시:
base_branches:
- "main"
- "develop"
- "production"
- "release/.*"현재 설정이 의도한 동작이 맞는지 확인 부탁드립니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.coderabbit.yml around lines 11 - 12, The base_branches setting currently
uses the broad pattern base_branches: ".*" which enables automatic reviews on
PRs targeting every branch; confirm whether this is intentional or narrow the
scope by replacing the ".*" pattern with explicit branch names or more specific
patterns (e.g., "main", "develop", "release/.*") in the base_branches entry to
limit review scope and resource usage; update the .coderabbit.yml base_branches
key accordingly and ensure any required branch patterns are listed explicitly.
| # macOS artefacts | ||
| .DS_Store | ||
|
|
||
| docs/ No newline at end of file |
There was a problem hiding this comment.
docs/ 전체 무시는 범위가 너무 넓을 수 있습니다
Line 61에서 docs/ 전체를 ignore하면, 수동으로 관리하는 문서(예: API 명세, 운영 가이드)까지 추적에서 빠질 수 있습니다. 생성 산출물만 제외하려는 목적이라면 /docs/build/, /docs/dist/처럼 하위 산출물 경로로 좁혀 주시는 것을 권장드립니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore at line 61, 현재 .gitignore의 "docs/" 항목은 전체 문서 디렉터리를 통째로 무시해 수동으로
관리하는 문서 파일까지 추적에서 빠지게 됩니다; .gitignore에서 "docs/" 항목을 찾아 전체 무시 대신 생성 산출물만 제외하도록
"/docs/build/", "/docs/dist/" 또는 특정 패턴("docs/**/*.html", "docs/**/*.pdf" 등)으로 좁혀
주고, 필요하면 수동 문서를 유지하려면 특정 파일을 예외 처리하는 방식(e.g., "!docs/README.md")을 사용하여 수동으로 관리하는
문서가 Git에 남도록 수정하세요.
Comment on lines
+38
to
+43
| User applicant = userService.findById(userNo); | ||
| Room room = roomService.findById(roomNo); | ||
|
|
||
| if (!applicant.getGender().equals(room.getGender())) { | ||
| throw new RestApiException(GENDER_MISMATCH); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
성별 검증 로직은 잘 추가되었으나, 비교 방식을 살짝 개선하실 수 있습니다.
두 필드 모두 enum(Gender)이고 DB 레벨에서 NOT NULL이 강제되므로, .equals(...) 대신 != 연산자를 사용하시면 가독성이 좋아지고 잠재적 NPE(한쪽이 null일 때 좌측에서 바로 NPE) 위험도 줄일 수 있습니다.
♻️ 제안 수정
- if (!applicant.getGender().equals(room.getGender())) {
+ if (applicant.getGender() != room.getGender()) {
throw new RestApiException(GENDER_MISMATCH);
}또한 현재 순서상 GENDER_MISMATCH가 CANNOT_APPLY_TO_OWN_ROOM보다 먼저 검사되는데, 호스트 본인이 자기 방에 지원하는 케이스에서 (같은 성별이므로) 기존처럼 CANNOT_APPLY_TO_OWN_ROOM이 나오게 되어 사용자 메시지 관점에서도 자연스럽습니다. 의도된 순서라면 그대로 두셔도 좋습니다.
📝 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
| User applicant = userService.findById(userNo); | |
| Room room = roomService.findById(roomNo); | |
| if (!applicant.getGender().equals(room.getGender())) { | |
| throw new RestApiException(GENDER_MISMATCH); | |
| } | |
| User applicant = userService.findById(userNo); | |
| Room room = roomService.findById(roomNo); | |
| if (applicant.getGender() != room.getGender()) { | |
| throw new RestApiException(GENDER_MISMATCH); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/project/dorumdorum/domain/room/application/usecase/ApplyRoomUseCase.java`
around lines 38 - 43, Replace the enum equality check using .equals(...) with an
identity comparison (use !=) between applicant.getGender() and room.getGender()
in ApplyRoomUseCase (after obtaining User via userService.findById(...) and Room
via roomService.findById(...)); change the condition that throws
RestApiException(GENDER_MISMATCH) from if
(!applicant.getGender().equals(room.getGender())) to if (applicant.getGender()
!= room.getGender()) to improve readability and avoid potential NPEs when
comparing the Gender enum values.
Comment on lines
+38
to
43
| if (roommateService.existsByUserNo(userNo)) { | ||
| throw new RestApiException(ALREADY_JOINED_USER); | ||
| } | ||
| Gender gender = userService.findById(userNo).getGender(); | ||
| Room room = roomService.create(userNo, gender, request); | ||
| roommateService.create(userNo, room, RoomRole.HOST); |
There was a problem hiding this comment.
이미 참여 중인지 확인하는 흐름을 원자적으로 보호해 주세요.
현재 existsByUserNo 확인과 roommateService.create 사이에 경쟁 조건이 있습니다. 동일 사용자의 동시 방 생성 요청이 둘 다 사전 검증을 통과하면, PR 목표와 달리 DB 제약 오류가 노출될 수 있습니다. 사용자/roommate 기준 pessimistic lock을 잡거나, roommate unique constraint 위반을 flush 시점에 특정해서 ALREADY_JOINED_USER로 변환해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/project/dorumdorum/domain/room/application/usecase/CreateRoomUseCase.java`
around lines 38 - 43, The check-and-create sequence in CreateRoomUseCase
(existsByUserNo -> roommateService.create) has a race; make it atomic by either
acquiring a DB pessimistic lock on the user/roommate row before checking (e.g.,
add/findByUserNoForUpdate in the roommate/user repository and use that lock in
CreateRoomUseCase or inside roommateService.create) or by keeping the optimistic
flow but catching the DB uniqueness violation when persisting the roommate
(catch DataIntegrityViolationException/PersistenceException thrown by
roommateService.create or its repository.save/flush) and translate that specific
unique-constraint error into throwing RestApiException(ALREADY_JOINED_USER);
update CreateRoomUseCase to use the locking variant or wrap the
roommateService.create call in a try/catch that maps unique-constraint failures
to ALREADY_JOINED_USER.
Comment on lines
+54
to
56
| @Column(nullable = false) | ||
| @Enumerated(EnumType.STRING) | ||
| private Gender gender; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# users 테이블 스키마/마이그레이션에 gender NOT NULL 반영이 있는지 확인
fd -e sql -e xml | xargs rg -nP -C2 '\busers?\b|\bgender\b' 2>/dev/null | rg -nP -i 'gender|users' | head -n 200Repository: DorumDorum/BE
Length of output: 728
🏁 Script executed:
# Check schema.sql file to see the full migration context
cat -n src/main/resources/schema.sql | head -30Repository: DorumDorum/BE
Length of output: 1287
🏁 Script executed:
# Look for migration files (Flyway, Liquibase)
fd -t f \( -name "*.sql" -o -name "*.xml" \) | head -20Repository: DorumDorum/BE
Length of output: 287
🏁 Script executed:
# Search for any backfill or UPDATE statements related to gender
rg -n "UPDATE.*gender|backfill|gender.*NULL" --type-listRepository: DorumDorum/BE
Length of output: 446
🏁 Script executed:
# Find all migration/sql files
fd -t f \( -name "*.sql" -o -name "*.xml" \)Repository: DorumDorum/BE
Length of output: 287
🏁 Script executed:
# Search for UPDATE statements or backfill logic related to gender
rg -n "UPDATE.*gender|INSERT.*gender" --type sqlRepository: DorumDorum/BE
Length of output: 39
🏁 Script executed:
# Check application configuration for schema initialization
fd -t f \( -name "application*.yml" -o -name "application*.properties" \) | xargs catRepository: DorumDorum/BE
Length of output: 287
🏁 Script executed:
# Find migration files more simply
find . -name "*.sql" -o -name "*.xml" | grep -E "(migration|flyway|liquibase)" | head -20Repository: DorumDorum/BE
Length of output: 39
🏁 Script executed:
# Search for UPDATE statements or backfill for gender
rg "UPDATE.*gender" -A 3Repository: DorumDorum/BE
Length of output: 39
🏁 Script executed:
# Check application properties
find . -name "application*.yml" -o -name "application*.properties"Repository: DorumDorum/BE
Length of output: 218
🏁 Script executed:
# Check Hibernate/JPA configuration in application.yml
cat ./src/main/resources/application.ymlRepository: DorumDorum/BE
Length of output: 4718
🏁 Script executed:
# Check prod config
cat ./src/main/resources/application-prod.ymlRepository: DorumDorum/BE
Length of output: 1290
🏁 Script executed:
# Look for any data initialization files
find ./src -name "*init*" -o -name "*migration*" -o -name "*data*" | head -20Repository: DorumDorum/BE
Length of output: 39
🏁 Script executed:
# Check if there's a separate migrations directory or if migrations are handled elsewhere
find ./src -type d -name "*migration*" -o -type d -name "*db*" -o -type d -name "*liquibase*" -o -type d -name "*flyway*"Repository: DorumDorum/BE
Length of output: 39
🏁 Script executed:
# Check pom.xml or gradle build file for migration framework dependencies
find . -name "pom.xml" -o -name "build.gradle" | head -5Repository: DorumDorum/BE
Length of output: 71
🏁 Script executed:
# Check if there's a separate schema or DB initialization configuration
rg -i "datasource|schema|initialization" ./src/main/resources/application*.yml -A 2Repository: DorumDorum/BE
Length of output: 659
🏁 Script executed:
# Check build.gradle dependencies
cat ./build.gradle | grep -i "flyway\|liquibase\|jpa\|hibernate" -A 2 -B 2Repository: DorumDorum/BE
Length of output: 816
🏁 Script executed:
# Check application-dev.yml for schema initialization settings
cat ./src/main/resources/application-dev.ymlRepository: DorumDorum/BE
Length of output: 997
🏁 Script executed:
# Look for schema initialization configuration (spring.sql.init)
rg "spring.sql.init|schema.sql|initialization" ./src/main/resources/ -A 2Repository: DorumDorum/BE
Length of output: 39
🏁 Script executed:
# Check if there's a deployment or migration documentation
find . -name "*.md" -o -name "MIGRATION*" -o -name "DEPLOYMENT*" -o -name "*.txt" | grep -i "migrat\|deploy\|setup" | head -10Repository: DorumDorum/BE
Length of output: 39
🏁 Script executed:
# Check if User entity is actually changed in this commit/PR
rg "nullable.*false" ./src/main/java --type java | grep -i genderRepository: DorumDorum/BE
Length of output: 39
운영 DB의 기존 레코드 마이그레이션 전략 확인이 필요해.
gender에 @Column(nullable = false) 제약을 추가한 점은 좋은 변경인데, 마이그레이션 전략에서 몇 가지 확인할 부분이 있어.
확인된 상황:
schema.sql에는ALTER TABLE users ALTER COLUMN gender SET NOT NULL;이 포함되어 있어- 하지만
application.yml(dev/prod 모두)에서ddl-auto: validate로 설정되어 있어서, 기존 데이터베이스에 이 스키마 변경이 자동으로 적용되지 않아 - 백필 마이그레이션 스크립트나 기존 NULL 값 처리 로직을 찾을 수 없어
배포 전에 다음을 확인해 줄래:
- (1) 운영 데이터에
gender가 NULL인 유저 레코드가 있는지 점검 - (2) 있다면 백필 마이그레이션 스크립트 준비 및 실행
- (3) 기존 데이터베이스에
NOT NULL제약을 적용하는 DDL 마이그레이션 프로세스 확인 (validate 모드에서는 schema.sql이 자동 실행되지 않으므로) - (4) 이후
User.getGender()를 사용하는ApplyRoomUseCase등에서 NPE가 발생하지 않도록 보장
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/project/dorumdorum/domain/user/domain/entity/User.java`
around lines 54 - 56, Confirm and fix migration for the new non-null constraint
on User.gender: (1) scan production DB for rows where gender IS NULL and
list/count them; (2) if any, add and run a backfill migration script
(referencing the schema change in schema.sql that does ALTER TABLE users ALTER
COLUMN gender SET NOT NULL) to populate a safe default or calculated value for
those rows before applying NOT NULL; (3) ensure your DDL migration process
(since application.yml uses ddl-auto: validate) will run the ALTER TABLE via
your migration tool (e.g., Flyway/Liquibase) rather than relying on Hibernate,
and add the NOT NULL DDL there; (4) add defensive handling where
User.getGender() is used (e.g., ApplyRoomUseCase and other callers) to avoid
NPEs until the backfill/constraint is in place (null checks or Optional) and
remove those safeguards once migration is complete.
Comment on lines
+19
to
+28
| ALTER TABLE IF EXISTS users | ||
| ALTER COLUMN gender SET NOT NULL; | ||
|
|
||
| DROP INDEX IF EXISTS idx_room_status_created; | ||
| DROP INDEX IF EXISTS idx_room_status_remaining_created; | ||
|
|
||
| CREATE INDEX IF NOT EXISTS idx_room_status_remaining_created | ||
| ON room (room_status, remaining, created_at DESC, room_no DESC); | ||
| CREATE INDEX IF NOT EXISTS idx_room_status_gender_created | ||
| ON room (room_status, gender, created_at DESC, room_no DESC); | ||
| CREATE INDEX IF NOT EXISTS idx_room_status_gender_remaining_created | ||
| ON room (room_status, gender, remaining ASC, created_at DESC, room_no DESC); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 설명: 성별 컬럼 추가/백필/NOT NULL 마이그레이션이 다른 파일에 있는지 확인합니다.
rg -n -C3 --pcre2 '(?i)(ALTER\s+TABLE\s+(IF\s+EXISTS\s+)?(room|users)|ADD\s+COLUMN\s+(IF\s+NOT\s+EXISTS\s+)?gender|UPDATE\s+(room|users).*gender|gender\s+SET\s+NOT\s+NULL|idx_room_status_gender)'Repository: DorumDorum/BE
Length of output: 2273
마이그레이션이 불완전해서 운영 배포 시 실패할 수 있어요. 성별 컬럼 추가 및 백필 단계를 추가해 주세요.
현재 스크립트는 몇 가지 중요한 단계가 누락되어 있습니다:
room테이블에gender컬럼이 아직 없으면 19번째 줄의 인덱스 생성이 실패해요- 기존
users.gender데이터 중 NULL 값이 있으면 20번째 줄에서 실패해요 room.gender백필 없이 NOT NULL 제약이 적용되면 새로운 레코드 삽입도 막혀요
코드에서 Room.gender가 nullable=false로 선언되어 있으니, 다음 순서대로 진행하는 게 안전해요:
room테이블에gender컬럼 추가- 호스트 유저의 성별로
room.gender백필 - NULL 값이 남아있는지 검증
- NOT NULL 제약 적용
- 인덱스 생성
제안 수정 예시
+ALTER TABLE IF EXISTS room
+ ADD COLUMN IF NOT EXISTS gender VARCHAR(255);
+
+UPDATE room r
+SET gender = u.gender
+FROM users u
+WHERE r.host_user_no = u.user_no
+ AND r.gender IS NULL;
+
+DO $$
+BEGIN
+ IF EXISTS (SELECT 1 FROM users WHERE gender IS NULL) THEN
+ RAISE EXCEPTION 'users.gender contains NULL rows; backfill before applying NOT NULL';
+ END IF;
+
+ IF EXISTS (SELECT 1 FROM room WHERE gender IS NULL) THEN
+ RAISE EXCEPTION 'room.gender contains NULL rows; backfill before applying NOT NULL';
+ END IF;
+END $$;
+
ALTER TABLE IF EXISTS users
ALTER COLUMN gender SET NOT NULL;
+
+ALTER TABLE IF EXISTS room
+ ALTER COLUMN gender SET NOT NULL;
DROP INDEX IF EXISTS idx_room_status_created;
DROP INDEX IF EXISTS idx_room_status_remaining_created;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/schema.sql` around lines 19 - 28, The migration is
incomplete: it adds NOT NULL and indexes on room.gender before the column exists
or is backfilled; fix by updating the migration to (1) ALTER TABLE room ADD
COLUMN gender TYPE (matching users.gender) nullable, (2) backfill room.gender
from users.gender using host user relation (e.g. join on host_user_id) for
existing rows, (3) verify no NULLs remain (raise/rollback if any), (4) ALTER
TABLE room ALTER COLUMN gender SET NOT NULL, and only then (5) create the
indexes idx_room_status_gender_created and
idx_room_status_gender_remaining_created; also ensure users.gender NULLs are
handled (update or validate) before ALTER COLUMN users.gender SET NOT NULL.
Comment on lines
+40
to
49
| @DisplayName("Should create and save room with gender from request") | ||
| void create_SavesRoom() { | ||
| RoomCreateRequest request = new RoomCreateRequest(RoomType.TYPE_1, 2, ResidencePeriod.SEMESTER, "title", null); | ||
| Room saved = Room.builder().roomNo("r1").hostUserNo("u1").build(); | ||
| Room saved = Room.builder().roomNo("r1").hostUserNo("u1").gender(Gender.MALE).build(); | ||
| when(roomRepository.save(any(Room.class))).thenReturn(saved); | ||
|
|
||
| Room result = service.create("u1", request); | ||
| Room result = service.create("u1", Gender.MALE, request); | ||
|
|
||
| assertThat(result).isEqualTo(saved); | ||
| verify(roomRepository).save(any(Room.class)); |
There was a problem hiding this comment.
저장 요청된 Room의 성별을 직접 검증해 주세요.
현재 테스트는 repository가 반환하는 saved 객체만 검증해서, RoomService.create가 실제 저장 엔티티에 Gender.MALE을 설정하지 않아도 통과할 수 있습니다.
테스트 검증 강화 예시
Room result = service.create("u1", Gender.MALE, request);
assertThat(result).isEqualTo(saved);
- verify(roomRepository).save(any(Room.class));
+ org.mockito.ArgumentCaptor<Room> roomCaptor =
+ org.mockito.ArgumentCaptor.forClass(Room.class);
+ verify(roomRepository).save(roomCaptor.capture());
+ assertThat(roomCaptor.getValue().getHostUserNo()).isEqualTo("u1");
+ assertThat(roomCaptor.getValue().getGender()).isEqualTo(Gender.MALE);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/project/dorumdorum/domain/room/unit/service/RoomServiceTest.java`
around lines 40 - 49, The test create_SavesRoom currently only asserts the
repository return value; update it to verify the actual Room passed into
roomRepository.save has gender set to Gender.MALE by capturing or matching the
argument from service.create("u1", Gender.MALE, request); use an ArgumentCaptor<
Room > or verify(roomRepository).save(argThat(r -> r.getGender() ==
Gender.MALE)) to assert the saved entity's gender, referencing
RoomService.create, RoomCreateRequest, roomRepository.save, Room and
Gender.MALE.
Comment on lines
+72
to
79
| when(roomRepository.findByCursor(any(Gender.class), any(), any(), any(), any(), anyInt())) | ||
| .thenReturn(expected); | ||
|
|
||
| List<FindRoomsResponse> result = service.searchByCursor( | ||
| request, null, null, null, 10 | ||
| Gender.MALE, request, null, null, null, 10 | ||
| ); | ||
|
|
||
| assertThat(result).isEqualTo(expected); |
There was a problem hiding this comment.
성별 필터 위임을 any()가 아니라 정확한 값으로 검증해 주세요.
any(Gender.class)를 사용하면 서비스가 잘못된 성별을 넘겨도 테스트가 통과합니다. 이 변경의 핵심 계약이 성별 필터링이므로 Gender.MALE 전달을 고정해서 검증하는 편이 안전합니다.
정확한 성별 전달 검증 예시
- when(roomRepository.findByCursor(any(Gender.class), any(), any(), any(), any(), anyInt()))
+ when(roomRepository.findByCursor(
+ org.mockito.ArgumentMatchers.eq(Gender.MALE),
+ any(), any(), any(), any(), anyInt()))
.thenReturn(expected);
List<FindRoomsResponse> result = service.searchByCursor(
Gender.MALE, request, null, null, null, 10
);
assertThat(result).isEqualTo(expected);
+ verify(roomRepository).findByCursor(
+ org.mockito.ArgumentMatchers.eq(Gender.MALE),
+ any(), any(), any(), any(), anyInt());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/project/dorumdorum/domain/room/unit/service/RoomServiceTest.java`
around lines 72 - 79, The test currently stubs roomRepository.findByCursor using
any(Gender.class) which allows incorrect genders to pass; change the stubbing to
assert the exact gender (Gender.MALE) is passed by replacing any(Gender.class)
with an equality matcher for Gender.MALE (e.g., eq(Gender.MALE)) in the
when(...) for roomRepository.findByCursor, and if present add/adjust a
verify(...) on roomRepository.findByCursor to explicitly check the Gender.MALE
argument; this ensures service.searchByCursor(Gender.MALE, request, ...) is
actually forwarding the correct gender.
Comment on lines
+100
to
+104
| assertThatThrownBy(() -> useCase.execute(userNo, roomNo, new JoinRoomRequest("intro", null))) | ||
| .isInstanceOf(RestApiException.class); | ||
|
|
||
| verify(roommateService, never()).isUserRoommate(anyString(), anyString()); | ||
| verify(roomRequestService, never()).create(anyString(), any(), any(), eq(Direction.USER_TO_ROOM)); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
GENDER_MISMATCH 오류 상태를 명시적으로 검증해 주세요.
현재는 RestApiException 타입만 확인하므로, 성별 불일치가 다른 room error로 매핑되어도 테스트가 통과합니다. GENDER_MISMATCH(ROOM016)까지 검증해야 이번 변경의 계약을 안전하게 고정할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/project/dorumdorum/domain/room/unit/usecase/ApplyRoomUseCaseTest.java`
around lines 100 - 104, The test only asserts the exception type from
useCase.execute but must also assert the specific error code for gender
mismatch; update the assertion that invokes useCase.execute(userNo, roomNo, new
JoinRoomRequest("intro", null)) to not only isInstanceOf(RestApiException.class)
but also verify the exception carries the GENDER_MISMATCH/ROOM016 error (e.g.,
inspect the RestApiException's error code or code getter and assert it equals
GENDER_MISMATCH or "ROOM016"), leaving the existing verify(...) calls unchanged.
Comment on lines
+50
to
+53
| assertThatThrownBy(() -> useCase.execute(userNo, request)) | ||
| .isInstanceOf(RestApiException.class); | ||
|
|
||
| verify(roomService, never()).create(any(), any(), any()); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
ALREADY_JOINED_USER 상태까지 검증해 주세요.
이 테스트는 현재 어떤 RestApiException이든 통과합니다. PR 목표가 DB 제약 오류 대신 ALREADY_JOINED_USER를 반환하는 것이므로, 예외 타입뿐 아니라 오류 상태도 함께 고정해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/project/dorumdorum/domain/room/unit/usecase/CreateRoomUseCaseTest.java`
around lines 50 - 53, The test CreateRoomUseCaseTest currently only asserts any
RestApiException from useCase.execute(userNo, request); change it to assert that
the thrown RestApiException contains the specific error state
ALREADY_JOINED_USER (e.g., by checking the exception's status/errorCode field or
using an assertion like hasFieldOrPropertyWithValue) and keep the existing
verify(roomService, never()).create(...) check; target the RestApiException
instance thrown by useCase.execute to ensure its error state equals
ALREADY_JOINED_USER.
[FIX] 방 관리 기능 개선 및 버그 수정 (+ 채팅 refactor)
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatMessageRepository.java (1)
23-25: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
deleteByChatRoomNo에도flushAutomatically = true를 추가해 주세요.이번 PR에서
decreaseUnreadCount(Line 13)에flushAutomatically = true를 추가했지만, 같은 파일의deleteByChatRoomNo(Line 23)는clearAutomatically = true만 남아 있어 패턴이 일치하지 않습니다.
flushAutomatically는 벌크 DML 실행 전에 퍼시스턴스 컨텍스트를 플러시할지 여부를 결정합니다.clearAutomatically만 사용할 경우, 아직 플러시되지 않은 변경 사항이 있을 때 컨텍스트를 클리어하면 저장되지 않은 변경이 유실될 수 있습니다. 이를 방지하기 위해flushAutomatically를 함께 사용하는 것이 안전합니다.
ChatRoomRepository.deleteByChatRoomNo와ChatRoomMemberRepository.deleteAllByChatRoom은 모두 두 속성을 함께 사용하고 있으므로,ChatMessageRepository도 동일한 패턴을 따르는 것이 좋습니다.♻️ 제안 수정
- `@Modifying`(clearAutomatically = true) + `@Modifying`(clearAutomatically = true, flushAutomatically = true) `@Query`("DELETE FROM ChatMessage m WHERE m.chatRoom.chatRoomNo = :chatRoomNo") void deleteByChatRoomNo(`@Param`("chatRoomNo") String chatRoomNo);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatMessageRepository.java` around lines 23 - 25, The `@Modifying` annotation on deleteByChatRoomNo should include flushAutomatically = true to match decreaseUnreadCount and other repositories; update the annotation on the method deleteByChatRoomNo in ChatMessageRepository to `@Modifying`(clearAutomatically = true, flushAutomatically = true) so the persistence context is flushed before executing the bulk DELETE and prevent lost unsaved changes.src/main/java/com/project/dorumdorum/domain/room/infra/repository/RoomRepositoryImpl.java (1)
209-264:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
findLikedRooms·findAppliedRooms·findMyRoom에 소프트 삭제 필터가 없습니다.이번 PR에서
findByCursor와findByCursorWithLateral에는room.deletedAt.isNull()필터가 추가되었지만, 동일 파일 내 다음 메서드들은 여전히 소프트 삭제된 방을 반환할 수 있습니다:
findMyRoom(Line 179): 삭제된 방이 "내 방"으로 여전히 표시될 수 있음findLikedRooms(Line 210): 삭제된 방이 좋아요 목록에 남아있을 수 있음findAppliedRooms(Line 238): 삭제된 방이 신청 목록에 남아있을 수 있음소프트 삭제 흐름이 이번 PR에서 완성되는 만큼, 이 메서드들에도
room.deletedAt.isNull()조건 추가를 검토해주세요.✅ 수정 예시 (findLikedRooms)
.where( roomLike.userNo.eq(userNo), - room.roomStatus.eq(RoomStatus.CONFIRM_PENDING) + room.roomStatus.eq(RoomStatus.CONFIRM_PENDING), + room.deletedAt.isNull() )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/project/dorumdorum/domain/room/infra/repository/RoomRepositoryImpl.java` around lines 209 - 264, The three repository methods findMyRoom, findLikedRooms, and findAppliedRooms are missing the soft-delete filter and may return deleted rooms; update each method’s QueryDSL where(...) clause to include room.deletedAt.isNull() alongside existing predicates (e.g., in findLikedRooms add room.deletedAt.isNull() with roomLike.userNo.eq(userNo) and room.roomStatus.eq(...), and similarly add it to findAppliedRooms and findMyRoom) so soft-deleted rooms are excluded from results.
♻️ Duplicate comments (1)
src/main/resources/schema.sql (1)
19-28:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win[이전 리뷰 미해결]
room.gender컬럼 추가·백필·NOT NULL 단계가 여전히 누락되어 있습니다.이전 리뷰에서 지적된 사항이 아직 반영되지 않았습니다. 현재 스크립트는 다음 문제가 존재합니다.
room테이블에gender컬럼이 없으면 Lines 25-28의 인덱스 생성이 실패합니다.users.gender에 NULL 값이 있으면 Lines 19-20의NOT NULL제약 적용이 실패합니다.room.gender에 대한NOT NULL제약 적용과 백필 단계가 없습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/schema.sql` around lines 19 - 28, Add missing schema-change steps so index creation won't fail: first ALTER TABLE room to ADD COLUMN IF NOT EXISTS gender with appropriate type/default, then backfill room.gender values for existing rows (e.g., UPDATE room SET gender = ... based on business rule) and finally ALTER TABLE room ALTER COLUMN gender SET NOT NULL; likewise ensure users.gender is backfilled for NULLs before executing ALTER TABLE users ALTER COLUMN gender SET NOT NULL; only after room.gender exists and is NOT NULL create the indexes idx_room_status_gender_created and idx_room_status_gender_remaining_created.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/project/dorumdorum/domain/chat/application/event/ChatWebSocketNotificationEvent.java`:
- Around line 5-8: ChatWebSocketNotificationEvent can receive null for
broadcasts or userNotifications and consumers like
ChatWebSocketNotificationEventListener call forEach which leads to NPE; add a
compact constructor for the record ChatWebSocketNotificationEvent that replaces
null broadcasts and userNotifications with empty immutable lists (e.g.,
List.of() or Collections.emptyList()) and optionally defensively copy inputs to
unmodifiable lists so event.userNotifications() and event.broadcasts() are never
null when listeners call forEach.
In
`@src/main/java/com/project/dorumdorum/domain/chat/application/event/ChatWebSocketNotificationEventListener.java`:
- Around line 16-23: The listener ChatWebSocketNotificationEventListener.handle
silently drops final failures because ChatWebSocketSendService.recoverSend
swallows exceptions after 3 retries; update the recovery path to record/emit a
failure metric (e.g., notificationFailureCounter.increment()) and/or trigger an
alerting/hand-off to an outbox for retry so failures are observable and
retriable, and add a brief comment in ChatWebSocketNotificationEventListener
explaining why fallbackExecution = false is used here while
NotificationRequestListener uses fallbackExecution = true (or change to true if
you prefer identical behavior) so future maintainers understand the mismatch
between notificationExecutor and fallbackExecution semantics.
In
`@src/main/java/com/project/dorumdorum/domain/chat/application/event/RoommateKickedEventListener.java`:
- Around line 58-61: The current logic in RoommateKickedEventListener builds
content using displayName derived from
userService.findById(event.kickedUserNo()) via kicked.getNickname() ?:
kicked.getName(), which can produce "null님이 퇴장했습니다." if kicked.getName() is
null; make this null-safe by ensuring displayName defaults to a safe fallback
(e.g., an empty string or "사용자") when both kicked.getNickname() and
kicked.getName() are null/blank before composing content so content never
includes the literal "null".
In
`@src/main/java/com/project/dorumdorum/domain/chat/domain/service/ChatRoomService.java`:
- Around line 70-72: Replace the custom deleting call in
ChatRoomService.deleteByChatRoomNo: instead of calling
chatRoomRepository.deleteByChatRoomNo(chatRoomNo) use the standard Spring Data
JPA method chatRoomRepository.deleteById(chatRoomNo) since chatRoomNo is the
entity primary key; update the method body in ChatRoomService to invoke
deleteById on chatRoomRepository to simplify and follow conventions.
In
`@src/main/java/com/project/dorumdorum/domain/chat/infra/websocket/ChatWebSocketSendService.java`:
- Around line 18-26: Narrow the Retryable scope on
ChatWebSocketSendService.broadcast and notifyUser: replace retryFor =
Exception.class with the specific transient messaging exception thrown by
SimpMessagingTemplate (e.g., MessageDeliveryException.class) and add recover =
"recoverSend" to the `@Retryable` so retries only occur for recoverable delivery
failures; then implement a corresponding recoverSend(...) `@Recover` method in the
same class to handle failed delivery (log/send fallback) for both broadcast and
notifyUser.
In
`@src/main/java/com/project/dorumdorum/domain/checklist/domain/entity/ChecklistBase.java`:
- Around line 126-178: The updateChecklist method on ChecklistBase currently
accepts ~25 positional parameters (bedtime, wakeUp, returnHomeTime,
lightsOutTime, hairDryer, otherNotes, etc.), which risks silent
parameter-swapping; create a single immutable DTO/record (e.g., ChecklistData)
that groups these related fields, change ChecklistBase.updateChecklist to accept
ChecklistData, update internal assignments to pull from ChecklistData, and
update both callers (RoomRuleMapper and UserChecklistMapper) to build and pass
ChecklistData instead of long positional lists so future additions remain
type-safe and maintainable.
In
`@src/main/java/com/project/dorumdorum/domain/room/application/usecase/DeleteRoomUseCase.java`:
- Line 6: DeleteRoomUseCase currently injects RoomLikeRepository directly
(imported RoomLikeRepository) which breaks layer boundaries because other
dependencies (roomRequestService, roomRuleService) are accessed via domain
services; create a RoomLikeService (or add a delegating method on an existing
service) that encapsulates roomLikeRepository and exposes a method like
deleteAllByRoom(Room room), then replace direct repository usage in
DeleteRoomUseCase with a call to that service method (remove RoomLikeRepository
injection and call roomLikeService.deleteAllByRoom(...) alongside existing
roomRequestService and roomRuleService calls).
In
`@src/main/java/com/project/dorumdorum/domain/room/domain/service/RoomRequestService.java`:
- Around line 60-62: Add the transactional annotation to explicitly declare the
delete operation is transactional: annotate the
RoomRequestService.deleteAllByRoom method with `@Transactional` (importing
org.springframework.transaction.annotation.Transactional) so that the call to
roomRequestRepository.deleteAllByRoom executes within its own transaction and
the intent is clear.
In
`@src/main/java/com/project/dorumdorum/domain/room/ui/spec/DeleteRoomApiSpec.java`:
- Around line 13-16: Update the `@Operation` description in DeleteRoomApiSpec to
list all deletion constraints enforced by DeleteRoomUseCase: require room owner
permission (NO_PERMISSION_ON_ROOM), disallow deleting completed/confirmed rooms
(CANNOT_DELETE_COMPLETED_ROOM), and disallow deletion when other members exist
(ROOM_HAS_MEMBERS); locate the annotation on the DeleteRoomApiSpec class/method
and expand the description string to explicitly mention each error code and
brief condition so API docs reflect all three failure cases.
In
`@src/main/java/com/project/dorumdorum/domain/roommate/infra/repository/RoommateRepositoryImpl.java`:
- Around line 50-53: 현재 WHERE 절에서 roommate.room.deletedAt.isNull()로 연관 엔티티를 직접
탐색하면 QueryDSL/JPA가 묵시적 크로스 조인을 생성하므로, RoommateRepositoryImpl의 쿼리에서
roommate.room을 명시적으로 조인하도록 변경하세요: 쿼리 빌더에서 roommate.room을 join 또는 leftJoin으로
명시(예: join(roommate.room, room) 또는 leftJoin(...))한 뒤 where 절에는
room.deletedAt.isNull()을 사용하도록 수정해 주세요; 관련 식별자는 roommate.room.deletedAt, QRoom
(room), RoommateRepositoryImpl의 해당 쿼리 메서드입니다.
- Around line 44-53: The soft-delete filter (roommate.deletedAt.isNull()) was
only added to findMyRoommates but is missing from other repository queries,
causing deleted Roommate rows to be returned; update RoommateRepositoryImpl's
implementations of findAllByUserNo, findByRoom, findByUserNoAndRoomNo,
existsByUserNoAndRoomNo, findByUserNoAndRoom, findByUserNo, and existsByUserNo
to include roommate.deletedAt.isNull() (and roommate.room.deletedAt.isNull()
where room is joined) in their where clauses or predicate builders so all reads
and existence checks consistently exclude soft-deleted records. Ensure exists*
methods apply the same predicates (not just select count) and reuse the same
predicate construction if possible.
In `@src/main/java/com/project/dorumdorum/global/config/WebSocketConfig.java`:
- Around line 25-29: The WebSocket endpoint registration
(registry.addEndpoint("/ws") using
setHandshakeHandler(jwtPrincipalHandshakeHandler) and
addInterceptors(jwtHandshakeInterceptor)) currently allows all origins via
setAllowedOriginPatterns("*"); change this to an explicit origin whitelist
(e.g., the same origins used in SecurityConfig such as http://localhost:3000,
http://localhost:5173, https://dorumdorum.com) by replacing the wildcard in
setAllowedOriginPatterns with the allowed origin strings so the /ws handshake
only accepts those trusted origins.
In
`@src/test/java/com/project/dorumdorum/domain/chat/unit/usecase/ChatWebSocketNotificationEventListenerTest.java`:
- Around line 25-40: Add two unit tests to
ChatWebSocketNotificationEventListenerTest to cover empty-list edge cases:
create a ChatWebSocketNotificationEvent with broadcasts non-empty and
userNotifications empty, call listener.handle(event) and verify
chatWebSocketSendService.notifyUser(...) is never() called while broadcast(...)
is called; then create an event with broadcasts empty and userNotifications
non-empty, call listener.handle(event) and verify
chatWebSocketSendService.broadcast(...) is never() called while notifyUser(...)
is called. Use the existing ChatWebSocketNotificationEvent.BroadcastTask and
UserNotifyTask constructors, listener.handle(event), and Mockito verify(...,
never())/verify(...) assertions to implement these tests.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/DeleteRoomPersistenceIntegrationTest.java`:
- Around line 192-194: The Awaitility call in
DeleteRoomPersistenceIntegrationTest using await().untilAsserted(...) lacks an
explicit timeout which can cause flakiness; update the call to specify a
reasonable atMost timeout (e.g., Duration.ofSeconds(5) or another value
appropriate for CI) on the await() before untilAsserted so the verification of
messagingTemplate.convertAndSendToUser(any(), eq("/queue/notification"), any())
with times(3) fails fast on timeout rather than relying on the default.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/KickRoommatePersistenceIntegrationTest.java`:
- Around line 169-173: The awaitility call in
KickRoommatePersistenceIntegrationTest currently uses await().untilAsserted(...)
without a timeout; update the assertion to specify an explicit timeout (e.g.,
await().atMost(Duration.ofSeconds(5)).untilAsserted(...)) to mirror
DeleteRoomPersistenceIntegrationTest and improve stability. Locate the await()
usage around the verifications of messagingTemplate.convertAndSend(...) and
messagingTemplate.convertAndSendToUser(...) and add an appropriate
atMost(Duration...) (or atMostSeconds) setting to the await() chain.
In
`@src/test/java/com/project/dorumdorum/domain/roommate/unit/infra/repository/RoommateRepositoryImplTest.java`:
- Around line 55-66: The test findMyRoommates_WhenNoResults_ReturnsEmptyList is
missing verification of the mock interactions; update the test to also verify
that queryFactory.select(...) and query.fetch() were invoked (e.g.,
verify(queryFactory).select(org.mockito.ArgumentMatchers.<Expression<?>>any());
verify(query).fetch();), so the test not only asserts the empty result but also
ensures the query chain in RoommateRepositoryImpl was executed.
In
`@src/test/java/com/project/dorumdorum/global/security/ChatRoomAuthorizationInterceptorTest.java`:
- Around line 84-102: The tests
preSend_SendWithoutSessionAttributes_ThrowsException and
preSend_SendWithoutUserNo_ThrowsException currently only assert exception type;
update them to also assert the specific error code/enum for the contract
(NOT_CHAT_ROOM_MEMBER) exposed by RestApiException. Locate the assertions around
interceptor.preSend(...) in those test methods and change them to capture the
thrown RestApiException and assert its error code/status (e.g., by using
assertThatThrownBy(...).isInstanceOf(RestApiException.class).satisfies(e ->
assertThat(((RestApiException)e).getErrorCode()).isEqualTo(NOT_CHAT_ROOM_MEMBER))
or equivalent) so the tests verify both type and the NOT_CHAT_ROOM_MEMBER code.
---
Outside diff comments:
In
`@src/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatMessageRepository.java`:
- Around line 23-25: The `@Modifying` annotation on deleteByChatRoomNo should
include flushAutomatically = true to match decreaseUnreadCount and other
repositories; update the annotation on the method deleteByChatRoomNo in
ChatMessageRepository to `@Modifying`(clearAutomatically = true,
flushAutomatically = true) so the persistence context is flushed before
executing the bulk DELETE and prevent lost unsaved changes.
In
`@src/main/java/com/project/dorumdorum/domain/room/infra/repository/RoomRepositoryImpl.java`:
- Around line 209-264: The three repository methods findMyRoom, findLikedRooms,
and findAppliedRooms are missing the soft-delete filter and may return deleted
rooms; update each method’s QueryDSL where(...) clause to include
room.deletedAt.isNull() alongside existing predicates (e.g., in findLikedRooms
add room.deletedAt.isNull() with roomLike.userNo.eq(userNo) and
room.roomStatus.eq(...), and similarly add it to findAppliedRooms and
findMyRoom) so soft-deleted rooms are excluded from results.
---
Duplicate comments:
In `@src/main/resources/schema.sql`:
- Around line 19-28: Add missing schema-change steps so index creation won't
fail: first ALTER TABLE room to ADD COLUMN IF NOT EXISTS gender with appropriate
type/default, then backfill room.gender values for existing rows (e.g., UPDATE
room SET gender = ... based on business rule) and finally ALTER TABLE room ALTER
COLUMN gender SET NOT NULL; likewise ensure users.gender is backfilled for NULLs
before executing ALTER TABLE users ALTER COLUMN gender SET NOT NULL; only after
room.gender exists and is NOT NULL create the indexes
idx_room_status_gender_created and idx_room_status_gender_remaining_created.
🪄 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.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8a56ff48-fbf9-438f-aad3-ff4dee9c4e75
📒 Files selected for processing (49)
src/main/java/com/project/dorumdorum/domain/chat/application/dto/response/NotificationMessage.javasrc/main/java/com/project/dorumdorum/domain/chat/application/dto/response/NotificationType.javasrc/main/java/com/project/dorumdorum/domain/chat/application/event/ChatWebSocketNotificationEvent.javasrc/main/java/com/project/dorumdorum/domain/chat/application/event/ChatWebSocketNotificationEventListener.javasrc/main/java/com/project/dorumdorum/domain/chat/application/event/RoomDeletedEventListener.javasrc/main/java/com/project/dorumdorum/domain/chat/application/event/RoommateKickedEventListener.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/entity/ChatRoom.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatMessageRepository.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatRoomMemberRepository.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatRoomRepository.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/service/ChatRoomMemberService.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/service/ChatRoomService.javasrc/main/java/com/project/dorumdorum/domain/chat/infra/websocket/ChatWebSocketSendService.javasrc/main/java/com/project/dorumdorum/domain/checklist/application/mapper/RoomRuleMapper.javasrc/main/java/com/project/dorumdorum/domain/checklist/application/mapper/UserChecklistMapper.javasrc/main/java/com/project/dorumdorum/domain/checklist/domain/entity/ChecklistBase.javasrc/main/java/com/project/dorumdorum/domain/checklist/domain/repository/RoomRuleRepository.javasrc/main/java/com/project/dorumdorum/domain/checklist/domain/service/RoomRuleService.javasrc/main/java/com/project/dorumdorum/domain/room/application/event/RoomDeletedEvent.javasrc/main/java/com/project/dorumdorum/domain/room/application/usecase/DeleteRoomUseCase.javasrc/main/java/com/project/dorumdorum/domain/room/application/usecase/KickRoommateUseCase.javasrc/main/java/com/project/dorumdorum/domain/room/domain/repository/RoomLikeRepository.javasrc/main/java/com/project/dorumdorum/domain/room/domain/repository/RoomRepository.javasrc/main/java/com/project/dorumdorum/domain/room/domain/repository/RoomRequestRepository.javasrc/main/java/com/project/dorumdorum/domain/room/domain/service/RoomRequestService.javasrc/main/java/com/project/dorumdorum/domain/room/domain/service/RoomService.javasrc/main/java/com/project/dorumdorum/domain/room/infra/repository/RoomRepositoryImpl.javasrc/main/java/com/project/dorumdorum/domain/room/ui/DeleteRoomController.javasrc/main/java/com/project/dorumdorum/domain/room/ui/spec/DeleteRoomApiSpec.javasrc/main/java/com/project/dorumdorum/domain/roommate/domain/service/RoommateService.javasrc/main/java/com/project/dorumdorum/domain/roommate/infra/repository/RoommateRepositoryImpl.javasrc/main/java/com/project/dorumdorum/global/config/WebSocketConfig.javasrc/main/java/com/project/dorumdorum/global/exception/code/status/RoomErrorStatus.javasrc/main/java/com/project/dorumdorum/global/security/ChatRoomAuthorizationInterceptor.javasrc/main/java/com/project/dorumdorum/global/security/JwtPrincipalHandshakeHandler.javasrc/main/java/com/project/dorumdorum/global/security/StompPrincipal.javasrc/main/resources/schema.sqlsrc/test/java/com/project/dorumdorum/domain/chat/unit/usecase/ChatWebSocketNotificationEventListenerTest.javasrc/test/java/com/project/dorumdorum/domain/chat/unit/usecase/RoomDeletedEventListenerTest.javasrc/test/java/com/project/dorumdorum/domain/chat/unit/usecase/RoommateKickedEventListenerTest.javasrc/test/java/com/project/dorumdorum/domain/checklist/unit/mapper/RoomRuleMapperTest.javasrc/test/java/com/project/dorumdorum/domain/checklist/unit/mapper/UserChecklistMapperTest.javasrc/test/java/com/project/dorumdorum/domain/room/integration/DeleteRoomPersistenceIntegrationTest.javasrc/test/java/com/project/dorumdorum/domain/room/integration/KickRoommatePersistenceIntegrationTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/usecase/DeleteRoomUseCaseTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/usecase/KickRoommateUseCaseTest.javasrc/test/java/com/project/dorumdorum/domain/roommate/unit/infra/repository/RoommateRepositoryImplTest.javasrc/test/java/com/project/dorumdorum/global/security/ChatRoomAuthorizationInterceptorTest.javasrc/test/java/com/project/dorumdorum/global/security/JwtPrincipalHandshakeHandlerTest.java
Comment on lines
+5
to
+8
| public record ChatWebSocketNotificationEvent( | ||
| List<BroadcastTask> broadcasts, | ||
| List<UserNotifyTask> userNotifications | ||
| ) { |
There was a problem hiding this comment.
null 리스트 전달 시 소비자 측에서 NPE 발생 가능
ChatWebSocketNotificationEventListener는 아래와 같이 forEach를 직접 호출합니다.
event.userNotifications().forEach(...)
event.broadcasts().forEach(...)생산자 측에서 어느 한 리스트에 null이 전달될 경우 NullPointerException이 발생합니다. compact 생성자로 방어 처리를 추가하는 것을 권장합니다.
🛡️ compact 생성자 추가 제안
+import java.util.Collections;
import java.util.List;
public record ChatWebSocketNotificationEvent(
List<BroadcastTask> broadcasts,
List<UserNotifyTask> userNotifications
) {
+ public ChatWebSocketNotificationEvent {
+ broadcasts = (broadcasts != null) ? broadcasts : Collections.emptyList();
+ userNotifications = (userNotifications != null) ? userNotifications : Collections.emptyList();
+ }
+
public record BroadcastTask(String chatRoomNo, Object payload) {}
public record UserNotifyTask(String userNo, Object payload) {}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/project/dorumdorum/domain/chat/application/event/ChatWebSocketNotificationEvent.java`
around lines 5 - 8, ChatWebSocketNotificationEvent can receive null for
broadcasts or userNotifications and consumers like
ChatWebSocketNotificationEventListener call forEach which leads to NPE; add a
compact constructor for the record ChatWebSocketNotificationEvent that replaces
null broadcasts and userNotifications with empty immutable lists (e.g.,
List.of() or Collections.emptyList()) and optionally defensively copy inputs to
unmodifiable lists so event.userNotifications() and event.broadcasts() are never
null when listeners call forEach.
Comment on lines
+16
to
+23
| @Async("notificationExecutor") | ||
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT, fallbackExecution = false) | ||
| public void handle(ChatWebSocketNotificationEvent event) { | ||
| event.userNotifications().forEach(task -> | ||
| chatWebSocketSendService.notifyUser(task.userNo(), task.payload())); | ||
| event.broadcasts().forEach(task -> | ||
| chatWebSocketSendService.broadcast(task.chatRoomNo(), task.payload())); | ||
| } |
There was a problem hiding this comment.
알림 최종 실패 시 무음 처리(Silent Drop) 위험이 있습니다.
ChatWebSocketSendService.recoverSend()는 3회 재시도 후 log.warn(...) 만 남기고 예외를 전파하지 않습니다. 이 리스너는 @Async + @TransactionalEventListener(AFTER_COMMIT) + fallbackExecution = false 조합으로 실행되므로, 재시도가 모두 소진되면 방 삭제 알림이 사용자에게 전달되지 않는 채로 조용히 드롭됩니다. 트랜잭션은 이미 커밋된 후이므로 호출자도 이 실패를 감지할 수 없습니다.
최소한 다음 중 하나를 권장합니다:
- 메트릭 카운터 증가 (예:
notificationFailureCounter.increment()) - 알림 재전송을 위한 별도 저장소(outbox 패턴) 도입
@Recover핸들러에서 알람(Alerting) 트리거
또한, fallbackExecution이 설정되지 않으면 이벤트가 활성 트랜잭션 없이 발행될 경우 무시됩니다. 동일한 notificationExecutor를 사용하는 NotificationRequestListener는 fallbackExecution = true로 설정되어 있습니다. 현재는 이 이벤트가 항상 활성 트랜잭션(BEFORE_COMMIT 리스너) 내에서 발행되므로 안전하지만, 두 리스너 간의 불일치에 대한 이유를 주석으로 명시하는 것이 좋습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/project/dorumdorum/domain/chat/application/event/ChatWebSocketNotificationEventListener.java`
around lines 16 - 23, The listener ChatWebSocketNotificationEventListener.handle
silently drops final failures because ChatWebSocketSendService.recoverSend
swallows exceptions after 3 retries; update the recovery path to record/emit a
failure metric (e.g., notificationFailureCounter.increment()) and/or trigger an
alerting/hand-off to an outbox for retry so failures are observable and
retriable, and add a brief comment in ChatWebSocketNotificationEventListener
explaining why fallbackExecution = false is used here while
NotificationRequestListener uses fallbackExecution = true (or change to true if
you prefer identical behavior) so future maintainers understand the mismatch
between notificationExecutor and fallbackExecution semantics.
Comment on lines
+58
to
+61
| User kicked = userService.findById(event.kickedUserNo()); | ||
| String displayName = (kicked.getNickname() != null && !kicked.getNickname().isBlank()) | ||
| ? kicked.getNickname() : kicked.getName(); | ||
| String content = displayName + "님이 퇴장했습니다."; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
표시 이름(displayName) null 안전성 검토가 필요합니다
kicked.getName()도 null일 경우 "null님이 퇴장했습니다."라는 메시지가 생성될 수 있습니다. User 엔티티에서 name 필드가 NotNull 제약이 있다면 문제없지만, 그렇지 않다면 방어적 처리를 고려해주세요.
♻️ 방어적 처리 제안
User kicked = userService.findById(event.kickedUserNo());
-String displayName = (kicked.getNickname() != null && !kicked.getNickname().isBlank())
- ? kicked.getNickname() : kicked.getName();
+String displayName = (kicked.getNickname() != null && !kicked.getNickname().isBlank())
+ ? kicked.getNickname()
+ : (kicked.getName() != null ? kicked.getName() : "사용자");
String content = displayName + "님이 퇴장했습니다.";📝 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
| User kicked = userService.findById(event.kickedUserNo()); | |
| String displayName = (kicked.getNickname() != null && !kicked.getNickname().isBlank()) | |
| ? kicked.getNickname() : kicked.getName(); | |
| String content = displayName + "님이 퇴장했습니다."; | |
| User kicked = userService.findById(event.kickedUserNo()); | |
| String displayName = (kicked.getNickname() != null && !kicked.getNickname().isBlank()) | |
| ? kicked.getNickname() | |
| : (kicked.getName() != null ? kicked.getName() : "사용자"); | |
| String content = displayName + "님이 퇴장했습니다."; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/project/dorumdorum/domain/chat/application/event/RoommateKickedEventListener.java`
around lines 58 - 61, The current logic in RoommateKickedEventListener builds
content using displayName derived from
userService.findById(event.kickedUserNo()) via kicked.getNickname() ?:
kicked.getName(), which can produce "null님이 퇴장했습니다." if kicked.getName() is
null; make this null-safe by ensuring displayName defaults to a safe fallback
(e.g., an empty string or "사용자") when both kicked.getNickname() and
kicked.getName() are null/blank before composing content so content never
includes the literal "null".
Comment on lines
+70
to
+72
| public void deleteByChatRoomNo(String chatRoomNo) { | ||
| chatRoomRepository.deleteByChatRoomNo(chatRoomNo); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# ChatRoomRepository의 deleteByChatRoomNo 정의 확인
rg -n -A4 "deleteByChatRoomNo" --type java
# ChatRoom 엔티티의 PK 필드 확인
rg -n "@Id" -A2 "ChatRoom" --type java | head -20Repository: DorumDorum/BE
Length of output: 4075
🏁 Script executed:
# ChatRoomRepository의 완전한 메서드 정의 확인
cat src/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatRoomRepository.javaRepository: DorumDorum/BE
Length of output: 1333
🏁 Script executed:
# ChatRoom 엔티티 파일 찾기
find . -name "ChatRoom.java" -type fRepository: DorumDorum/BE
Length of output: 136
🏁 Script executed:
# ChatRoom 엔티티의 `@Id` 필드 확인
cat src/main/java/com/project/dorumdorum/domain/chat/domain/entity/ChatRoom.javaRepository: DorumDorum/BE
Length of output: 1700
deleteById 사용을 고려해볼 수 있어요.
chatRoomRepository.deleteByChatRoomNo는 @Modifying 커스텀 쿼리로 구현되어 있어서 트랜잭션 안전성은 문제없습니다. 다만 chatRoomNo가 ChatRoom의 기본 키이기 때문에, deleteById(chatRoomNo)를 사용하는 것이 더 관례적입니다.
public void deleteByChatRoomNo(String chatRoomNo) {
chatRoomRepository.deleteById(chatRoomNo);
}
이렇게 변경하면 Spring Data JPA의 표준 메서드를 활용해서 코드가 더 간결해질 거예요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/project/dorumdorum/domain/chat/domain/service/ChatRoomService.java`
around lines 70 - 72, Replace the custom deleting call in
ChatRoomService.deleteByChatRoomNo: instead of calling
chatRoomRepository.deleteByChatRoomNo(chatRoomNo) use the standard Spring Data
JPA method chatRoomRepository.deleteById(chatRoomNo) since chatRoomNo is the
entity primary key; update the method body in ChatRoomService to invoke
deleteById on chatRoomRepository to simplify and follow conventions.
Comment on lines
+18
to
+26
| @Retryable(retryFor = Exception.class, maxAttempts = 3, backoff = @Backoff(delay = 1000, multiplier = 2)) | ||
| public void broadcast(String chatRoomNo, Object payload) { | ||
| messagingTemplate.convertAndSend("/topic/chat-room/" + chatRoomNo, payload); | ||
| } | ||
|
|
||
| @Retryable(retryFor = Exception.class, maxAttempts = 3, backoff = @Backoff(delay = 1000, multiplier = 2)) | ||
| public void notifyUser(String userNo, Object payload) { | ||
| messagingTemplate.convertAndSendToUser(userNo, "/queue/notification", payload); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
retryFor = Exception.class의 재시도 범위가 지나치게 광범위합니다
Exception.class를 지정하면 NullPointerException, ClassCastException처럼 재시도해도 해결되지 않는 프로그래밍 오류까지 3회 재시도하게 됩니다. 각 시도 간 최대 1 + 2 = 3초의 지연이 발생하여 이벤트 리스너 쓰레드가 불필요하게 블로킹될 수 있습니다.
Spring Retry 공식 문서도 "Failures are inherently either retryable or not — if the same exception is always going to be thrown from the business logic, it does not help to retry it"라고 권고하고 있습니다.
SimpMessagingTemplate이 발생시키는 일시적 전송 실패(MessageDeliveryException)와 같은 구체적인 예외 타입으로 범위를 좁히는 것을 권장합니다.
♻️ 재시도 예외 타입 한정 제안
-@Retryable(retryFor = Exception.class, maxAttempts = 3, backoff = `@Backoff`(delay = 1000, multiplier = 2))
+@Retryable(retryFor = MessageDeliveryException.class, maxAttempts = 3, backoff = `@Backoff`(delay = 1000, multiplier = 2), recover = "recoverSend")
public void broadcast(String chatRoomNo, Object payload) {
messagingTemplate.convertAndSend("/topic/chat-room/" + chatRoomNo, payload);
}
-@Retryable(retryFor = Exception.class, maxAttempts = 3, backoff = `@Backoff`(delay = 1000, multiplier = 2))
+@Retryable(retryFor = MessageDeliveryException.class, maxAttempts = 3, backoff = `@Backoff`(delay = 1000, multiplier = 2), recover = "recoverSend")
public void notifyUser(String userNo, Object payload) {
messagingTemplate.convertAndSendToUser(userNo, "/queue/notification", payload);
}💡
recover = "recoverSend"명시 바인딩은 향후 동일 클래스에 또 다른@Retryable/@Recover메서드가 추가될 때 충돌 없이 올바른 복구 메서드를 찾을 수 있도록 해 줍니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/project/dorumdorum/domain/chat/infra/websocket/ChatWebSocketSendService.java`
around lines 18 - 26, Narrow the Retryable scope on
ChatWebSocketSendService.broadcast and notifyUser: replace retryFor =
Exception.class with the specific transient messaging exception thrown by
SimpMessagingTemplate (e.g., MessageDeliveryException.class) and add recover =
"recoverSend" to the `@Retryable` so retries only occur for recoverable delivery
failures; then implement a corresponding recoverSend(...) `@Recover` method in the
same class to handle failed delivery (log/send fallback) for both broadcast and
notifyUser.
Comment on lines
+25
to
+40
| @Test | ||
| @DisplayName("개인 알림을 브로드캐스트보다 먼저 보낸다") | ||
| void handle_SendsUserNotificationBeforeBroadcast() { | ||
| Object userPayload = new Object(); | ||
| Object broadcastPayload = new Object(); | ||
| ChatWebSocketNotificationEvent event = new ChatWebSocketNotificationEvent( | ||
| List.of(new ChatWebSocketNotificationEvent.BroadcastTask("chat-room-1", broadcastPayload)), | ||
| List.of(new ChatWebSocketNotificationEvent.UserNotifyTask("user-1", userPayload)) | ||
| ); | ||
|
|
||
| listener.handle(event); | ||
|
|
||
| InOrder inOrder = inOrder(chatWebSocketSendService); | ||
| inOrder.verify(chatWebSocketSendService).notifyUser("user-1", userPayload); | ||
| inOrder.verify(chatWebSocketSendService).broadcast("chat-room-1", broadcastPayload); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
엣지 케이스 테스트 커버리지를 보완해 주세요.
현재 테스트는 각각 1개의 태스크가 있는 정상 경로와 순서만 검증합니다. 다음 케이스들이 누락되어 있습니다:
userNotifications가 비어 있을 때 →notifyUser가 호출되지 않아야 함broadcasts가 비어 있을 때 →broadcast가 호출되지 않아야 함
💡 추가 테스트 예시
`@Test`
`@DisplayName`("userNotifications가 비어 있으면 notifyUser를 호출하지 않는다")
void handle_WhenNoUserNotifications_DoesNotCallNotifyUser() {
ChatWebSocketNotificationEvent event = new ChatWebSocketNotificationEvent(
List.of(new ChatWebSocketNotificationEvent.BroadcastTask("chat-room-1", new Object())),
List.of()
);
listener.handle(event);
verify(chatWebSocketSendService, never()).notifyUser(any(), any());
verify(chatWebSocketSendService).broadcast(eq("chat-room-1"), any());
}
`@Test`
`@DisplayName`("broadcasts가 비어 있으면 broadcast를 호출하지 않는다")
void handle_WhenNoBroadcasts_DoesNotCallBroadcast() {
ChatWebSocketNotificationEvent event = new ChatWebSocketNotificationEvent(
List.of(),
List.of(new ChatWebSocketNotificationEvent.UserNotifyTask("user-1", new Object()))
);
listener.handle(event);
verify(chatWebSocketSendService, never()).broadcast(any(), any());
verify(chatWebSocketSendService).notifyUser(eq("user-1"), any());
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/project/dorumdorum/domain/chat/unit/usecase/ChatWebSocketNotificationEventListenerTest.java`
around lines 25 - 40, Add two unit tests to
ChatWebSocketNotificationEventListenerTest to cover empty-list edge cases:
create a ChatWebSocketNotificationEvent with broadcasts non-empty and
userNotifications empty, call listener.handle(event) and verify
chatWebSocketSendService.notifyUser(...) is never() called while broadcast(...)
is called; then create an event with broadcasts empty and userNotifications
non-empty, call listener.handle(event) and verify
chatWebSocketSendService.broadcast(...) is never() called while notifyUser(...)
is called. Use the existing ChatWebSocketNotificationEvent.BroadcastTask and
UserNotifyTask constructors, listener.handle(event), and Mockito verify(...,
never())/verify(...) assertions to implement these tests.
Comment on lines
+192
to
+194
| await().untilAsserted(() -> | ||
| verify(messagingTemplate, times(3)) | ||
| .convertAndSendToUser(any(), eq("/queue/notification"), any())); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Awaitility 타임아웃 명시를 권장합니다
await().untilAsserted(...)에 명시적인 타임아웃이 설정되어 있지 않아 CI 환경에서 불안정한 테스트(flaky test)가 될 수 있습니다. 기본값(10초)에 의존하기보다 테스트 의도에 맞는 적절한 타임아웃을 명시해주시는 것이 좋습니다.
♻️ 타임아웃 명시 제안
+import java.util.concurrent.TimeUnit;
...
-await().untilAsserted(() ->
+await().atMost(5, TimeUnit.SECONDS).untilAsserted(() ->
verify(messagingTemplate, times(3))
.convertAndSendToUser(any(), eq("/queue/notification"), any()));📝 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
| await().untilAsserted(() -> | |
| verify(messagingTemplate, times(3)) | |
| .convertAndSendToUser(any(), eq("/queue/notification"), any())); | |
| await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> | |
| verify(messagingTemplate, times(3)) | |
| .convertAndSendToUser(any(), eq("/queue/notification"), any())); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/DeleteRoomPersistenceIntegrationTest.java`
around lines 192 - 194, The Awaitility call in
DeleteRoomPersistenceIntegrationTest using await().untilAsserted(...) lacks an
explicit timeout which can cause flakiness; update the call to specify a
reasonable atMost timeout (e.g., Duration.ofSeconds(5) or another value
appropriate for CI) on the await() before untilAsserted so the verification of
messagingTemplate.convertAndSendToUser(any(), eq("/queue/notification"), any())
with times(3) fails fast on timeout rather than relying on the default.
Comment on lines
+169
to
+173
| await().untilAsserted(() -> { | ||
| verify(messagingTemplate).convertAndSend( | ||
| eq("/topic/chat-room/" + chatRoom.getChatRoomNo()), any(Object.class)); | ||
| verify(messagingTemplate).convertAndSendToUser(eq(MEMBER_NO), eq("/queue/notification"), any()); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Awaitility 타임아웃 명시를 권장합니다
DeleteRoomPersistenceIntegrationTest와 마찬가지로 명시적인 타임아웃 설정이 없습니다. 비동기 검증의 안정성을 위해 타임아웃을 명시해주세요.
♻️ 타임아웃 명시 제안
+import java.util.concurrent.TimeUnit;
...
-await().untilAsserted(() -> {
+await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> {
verify(messagingTemplate).convertAndSend(
eq("/topic/chat-room/" + chatRoom.getChatRoomNo()), any(Object.class));
verify(messagingTemplate).convertAndSendToUser(eq(MEMBER_NO), eq("/queue/notification"), any());
});📝 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
| await().untilAsserted(() -> { | |
| verify(messagingTemplate).convertAndSend( | |
| eq("/topic/chat-room/" + chatRoom.getChatRoomNo()), any(Object.class)); | |
| verify(messagingTemplate).convertAndSendToUser(eq(MEMBER_NO), eq("/queue/notification"), any()); | |
| }); | |
| await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> { | |
| verify(messagingTemplate).convertAndSend( | |
| eq("/topic/chat-room/" + chatRoom.getChatRoomNo()), any(Object.class)); | |
| verify(messagingTemplate).convertAndSendToUser(eq(MEMBER_NO), eq("/queue/notification"), any()); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/KickRoommatePersistenceIntegrationTest.java`
around lines 169 - 173, The awaitility call in
KickRoommatePersistenceIntegrationTest currently uses await().untilAsserted(...)
without a timeout; update the assertion to specify an explicit timeout (e.g.,
await().atMost(Duration.ofSeconds(5)).untilAsserted(...)) to mirror
DeleteRoomPersistenceIntegrationTest and improve stability. Locate the await()
usage around the verifications of messagingTemplate.convertAndSend(...) and
messagingTemplate.convertAndSendToUser(...) and add an appropriate
atMost(Duration...) (or atMostSeconds) setting to the await() chain.
Comment on lines
+55
to
+66
| @Test | ||
| @DisplayName("룸메이트가 없으면 빈 리스트 반환") | ||
| @SuppressWarnings({"unchecked", "rawtypes"}) | ||
| void findMyRoommates_WhenNoResults_ReturnsEmptyList() { | ||
| when(queryFactory.select(org.mockito.ArgumentMatchers.<Expression<?>>any())).thenReturn(query); | ||
| when(query.fetch()).thenReturn(List.of()); | ||
|
|
||
| RoommateRepositoryImpl repository = new RoommateRepositoryImpl(queryFactory); | ||
| List<MyRoommateResponse> result = repository.findMyRoommates("u1"); | ||
|
|
||
| assertThat(result).isEmpty(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
새 테스트에 verify 호출이 누락되어 있습니다.
기존 테스트(findMyRoommates_ReturnsFetchedResults)는 queryFactory.select()와 query.fetch() 호출을 verify로 검증하는 반면, 이번에 추가된 테스트는 결과값만 단언하고 쿼리 체인 실행 여부는 전혀 검증하지 않습니다. 예를 들어 fetch() 호출이 누락되어도 이 테스트는 통과됩니다.
✅ 개선 제안
void findMyRoommates_WhenNoResults_ReturnsEmptyList() {
when(queryFactory.select(org.mockito.ArgumentMatchers.<Expression<?>>any())).thenReturn(query);
when(query.fetch()).thenReturn(List.of());
RoommateRepositoryImpl repository = new RoommateRepositoryImpl(queryFactory);
List<MyRoommateResponse> result = repository.findMyRoommates("u1");
assertThat(result).isEmpty();
+ verify(queryFactory).select(org.mockito.ArgumentMatchers.<Expression<?>>any());
+ verify(query).fetch();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/project/dorumdorum/domain/roommate/unit/infra/repository/RoommateRepositoryImplTest.java`
around lines 55 - 66, The test findMyRoommates_WhenNoResults_ReturnsEmptyList is
missing verification of the mock interactions; update the test to also verify
that queryFactory.select(...) and query.fetch() were invoked (e.g.,
verify(queryFactory).select(org.mockito.ArgumentMatchers.<Expression<?>>any());
verify(query).fetch();), so the test not only asserts the empty result but also
ensures the query chain in RoommateRepositoryImpl was executed.
Comment on lines
+84
to
+102
| @Test | ||
| @DisplayName("SEND — sessionAttributes가 없으면 NOT_CHAT_ROOM_MEMBER 예외") | ||
| void preSend_SendWithoutSessionAttributes_ThrowsException() { | ||
| StompHeaderAccessor accessor = StompHeaderAccessor.create(StompCommand.SEND); | ||
| accessor.setDestination("/app/chat-room/cr-1/send"); | ||
| Message<byte[]> message = MessageBuilder.createMessage(new byte[0], accessor.getMessageHeaders()); | ||
|
|
||
| assertThatThrownBy(() -> interceptor.preSend(message, channel)) | ||
| .isInstanceOf(RestApiException.class); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("SEND — sessionAttributes에 userNo가 없으면 NOT_CHAT_ROOM_MEMBER 예외") | ||
| void preSend_SendWithoutUserNo_ThrowsException() { | ||
| Message<byte[]> message = sendMessage("/app/chat-room/cr-1/send", new HashMap<>()); | ||
|
|
||
| assertThatThrownBy(() -> interceptor.preSend(message, channel)) | ||
| .isInstanceOf(RestApiException.class); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
예외 타입만 보지 말고 에러 코드까지 같이 고정해 주세요.
이 구간은 NOT_CHAT_ROOM_MEMBER 계약이 핵심인데, 지금처럼 RestApiException 타입만 확인하면 다른 사유의 예외로 바뀌어도 테스트가 통과할 수 있습니다. 프로젝트에서 노출하는 accessor 기준으로 에러 코드/상태까지 함께 검증해 두시는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/project/dorumdorum/global/security/ChatRoomAuthorizationInterceptorTest.java`
around lines 84 - 102, The tests
preSend_SendWithoutSessionAttributes_ThrowsException and
preSend_SendWithoutUserNo_ThrowsException currently only assert exception type;
update them to also assert the specific error code/enum for the contract
(NOT_CHAT_ROOM_MEMBER) exposed by RestApiException. Locate the assertions around
interceptor.preSend(...) in those test methods and change them to capture the
thrown RestApiException and assert its error code/status (e.g., by using
assertThatThrownBy(...).isInstanceOf(RestApiException.class).satisfies(e ->
assertThat(((RestApiException)e).getErrorCode()).isEqualTo(NOT_CHAT_ROOM_MEMBER))
or equivalent) so the tests verify both type and the NOT_CHAT_ROOM_MEMBER code.
6 tasks
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
2 participants
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.
📝 Pull Request Template
📌 제목
✅ PR 체크리스트
📜 기타
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정 / 동작 변경
인프라 / DB