[COZY-492] feat: 방 추천에 대학교, 성별 필터링 추가 #252
Conversation
eple0329
requested review from
genius00hwan,
veronees and
suuu0719
as code owners
Walkthrough이 변경 사항은 두 개의 주요 Java 클래스에 대한 수정을 포함합니다. Sequence DiagramsequenceDiagram
participant Member
participant RoomRecommendService
participant MateRepository
Member->>RoomRecommendService: getRoomList() 요청
RoomRecommendService->>MateRepository: findAllByRoomIdListAndIsRoomManagerAndEntryStatus()
MateRepository-->>RoomRecommendService: Mate 엔티티 목록 반환
RoomRecommendService->>RoomRecommendService: 대학 및 성별 필터링
RoomRecommendService-->>Member: 필터링된 방 목록 반환
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 0
🧹 Nitpick comments (1)
src/main/java/com/cozymate/cozymate_server/domain/mate/repository/MateRepository.java (1)
116-125:FETCH JOIN시 중복 레코드 문제에 주의하세요
JOIN FETCH를 사용해room,member,university정보를 한 번에 불러오는 것은 N+1 문제를 줄이는 데 유용합니다. 다만DISTINCT키워드를 사용하지 않으면, 다대일 관계나 다대다 관계에서 중복 레코드가 발생할 수 있으니 필요할 경우SELECT DISTINCT mt FROM ~형태를 고려해 보세요. 또한roomIdList가 매우 클 때는 쿼리 성능 테스트를 통해 부하를 점검하면 좋겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/cozymate/cozymate_server/domain/mate/repository/MateRepository.java(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomRecommendService.java(2 hunks)
🔇 Additional comments (2)
src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomRecommendService.java (2)
29-36:@Slf4j어노테이션 도입을 환영합니다
로그를 남길 수 있어 운영 및 디버깅에 도움이 됩니다. 로깅 레벨 설정이나 민감정보 마스킹에 유의해 주세요.
116-129:stream().forEach()에서roomList를 직접 제거하는 로직 수정 검토
필터 조건(대학교/성별)에 맞지 않는Room을 제거하기 위해managerList.stream()...forEach(roomList::remove())를 사용하는 패턴은, 반복 중에 리스트에서 요소를 제거하기 때문에 유지보수성 측면에서 혼동을 줄 수 있습니다. 다음과 같은 방안을 고려해 보세요.
- 제거 대상
Room을 별도의 집합(Set)으로 수집 후, 한 번에 제거하기- 단일 스트림 체이닝으로 필터 후
collect(...)하는 방식으로roomList를 재할당또한
manager.getMember().getUniversity()혹은manager.getMember().getGender()가null일 수 있는지 확인하여 널 체크 로직을 추가할 필요가 없는지 점검해 보세요.필요하다면 다음 스크립트를 통해
Member가 항상null이 아닌university와gender를 가진다고 보장되는지 확인할 수 있습니다:
| @Query(""" | ||
| SELECT mt FROM Mate mt | ||
| JOIN FETCH mt.room r | ||
| JOIN FETCH mt.member m | ||
| JOIN FETCH m.university u | ||
| WHERE mt.room.id IN :roomIdList | ||
| AND mt.isRoomManager = :isRoomManager | ||
| AND mt.entryStatus = :entryStatus | ||
| """) | ||
| List<Mate> findAllByRoomIdListAndIsRoomManagerAndEntryStatus(List<Long> roomIdList, boolean isRoomManager, EntryStatus entryStatus); |
There was a problem hiding this comment.
가져온 방의 id를 바탕으로 방장의 정보를 가져옵니다.
방장의 roomId가 무엇인지 알아야하기 때문에 room과 join을 했고
대학 id가 무엇인지 알아야하기 때문에 member university와 join을 했습니다.
| // 대학 필터링 | ||
| managerList.stream() | ||
| .filter(manager -> !manager.getMember().getUniversity().getId().equals(member.getUniversity().getId())) | ||
| .forEach(manager -> roomList.remove(manager.getRoom())); | ||
|
|
||
| // 성별 필터링 | ||
| managerList.stream() | ||
| .filter(manager -> !manager.getMember().getGender().equals(member.getGender())) | ||
| .forEach(manager -> roomList.remove(manager.getRoom())); |
There was a problem hiding this comment.
단순 필터링입니다.
좀 더 읽기 편하라고 이렇게 작성했습니다.
어차피 이후에 room에 추가되면 삭제될 코드이기도 하고요
⚒️develop의 최신 커밋을 pull 받았나요?
넵
#️⃣ 작업 내용
Room 엔티티에 university와 gender 값이 들어오길 기다리다가, 일단 배포를 하게 되었으니 Mate를 통해서 필터링하도록 했습니다.
이후에 추가되게 되면 다시 원상복구 시키고, Room 엔티티를 참조하도록 하겠습니다.
동작 확인
기존에 성별이 동일할 때 검색 결과 -> 우심방 나옴
델로롱을 여자로 변경 (우심방의 방장임)
이후 검색 결과 -> 우심방이 나오지 않음
기존에 대학이 같을 때 검색 결과 -> 우심방 나옴
델로롱을 인하대에서 다른 학교로 변경 (우심방의 방장임)
이후 검색 결과 -> 우심방이 나오지 않음
💬 리뷰 요구사항(선택)
개발하면서 점점 많이 알아가고 있는 중입니다.
근데 Member와 MemberStat의 연관 관계의 주인이 MemberStat이라 불필요한 N+1 쿼리가 계속 날라가게 됩니다.
이 연관관계는 주종 관계를 바꾸는게 좋을 것 같습니다. (Member가 주인이고, MemberStat에서 mapped By로 참조하는 식으로)
Summary by CodeRabbit
새로운 기능
개선 사항