-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[COZY-527] fix: RoomHashtag 관계 변경 #281
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough이번 변경사항은 여러 도메인 및 서비스 계층 전반에 걸쳐 다양한 수정이 이루어졌음을 보여줍니다.
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant RQS as RoomQueryService
participant Repo as Repository
Client->>RQS: getRoomById(roomId, ...)
RQS->>RQS: validateRoom()
RQS->>RQS: validateRoomAccess()
RQS->>RQS: processRoomDetails()
RQS->>Repo: 조회 요청 (Joined Mates 등)
Repo-->>RQS: 관련 데이터 전달
RQS->>RQS: calculateLifestyleMatchRates()
RQS->>RQS: getRoomHashtagsMap() & extractHashtags()
RQS-->>Client: RoomDetail 응답 DTO 전달
sequenceDiagram
participant Client as 클라이언트
participant RRS as RoomRecommendService
participant RHRepo as RoomHashtagRepository
participant RC as RoomRecommendConverter
Client->>RRS: getRecommendationList(member, size, page, sortType)
RRS->>RHRepo: findByRoomIds(roomIds) 호출
RHRepo-->>RRS: roomHashtagsMap 반환
RRS->>RRS: buildRoomRecommendationResponseList(..., roomHashtagsMap)
RRS->>RC: createRoomRecommendationResponse(...) 호출
RC-->>RRS: RoomRecommendationResponseDTO 반환
RRS-->>Client: 추천 응답 DTO 전달
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (10)
src/main/java/com/cozymate/cozymate_server/domain/roomfavorite/service/RoomFavoriteQueryService.java (1)
100-105
: 스트림 연산을 단순화할 수 있습니다.현재 구현은 올바르게 작동하지만, 다음과 같이 더 간단하게 작성할 수 있습니다:
-Map<Long, List<String>> roomHashtagsMap = roomHashtagRepository.findByRoomIds(responseRoomList.stream().map(Room::getId).toList()) - .stream() - .collect(Collectors.groupingBy( - roomHashtag -> roomHashtag.getRoom().getId(), - Collectors.mapping(roomHashtag -> roomHashtag.getHashtag().getHashtag(), Collectors.toList()) - )); +Map<Long, List<String>> roomHashtagsMap = roomHashtagRepository.findByRoomIds(responseRoomList.stream().map(Room::getId).toList()) + .stream() + .collect(Collectors.groupingBy( + roomHashtag -> roomHashtag.getRoom().getId(), + Collectors.mapping(hashtag -> hashtag.getHashtag().getHashtag(), Collectors.toList()) + ));src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomQueryService.java (7)
47-47
: RoomHashtagRepository 필드 주입에 대한 구조적 고려
해시태그 관련 로직이 해당 서비스에 직접적으로 주입되고 있습니다. 추후 유지보수나 확장을 위해, 해시태그 로직을 전담하는 별도 서비스/도메인 계층으로 분리해 두면 보다 유연한 설계가 될 수 있습니다.
79-80
: 권한 검증 메서드 사용 시 네이밍 일관성 확인
validateRoomMember
는 JOINED 상태를 필수로 검사하는 메서드입니다. 향후 ‘대기 중(PENDING)’이거나 초대(INVITED) 상태도 포함된 확장 검증이 필요하다면, 메서드명을 좀 더 포괄적으로 변경하거나 별도의 메서드로 분리하는 것을 고려해 볼 수 있습니다.
130-130
: JOINED 메이트 조회 시 성능 고려
mateRepository.findJoinedMatesWithMemberAndStats(room.getId())
를 사용하면 N+1 문제를 상당 부분 해결할 수 있습니다. 다만 이곳에서 가져오는memberStat
가 방 상세에서 과도하게 사용되지 않는다면, 상황에 따라 select 절을 필요한 컬럼만 가져오는 형태로 조절할 수도 있습니다.
183-186
: 즐겨찾기 로직 최소 반환값 검토
orElse(0L)
로 즐겨찾기 관련 ID가 없으면 0을 리턴하고 있습니다. 추후 0L 대신 null을 사용할 경우에는 클라이언트 사이드에서 처리가 달라질 수 있으므로, 부가적인 요구사항(예: 즐겨찾기 미등록 상태를 구분하는 방식)에 따라 반환값을 명확히 정의해두세요.
189-192
: PRIVATE 방 검증 로직
room.getRoomType() == RoomType.PRIVATE
검사 후validateRoomMember
를 호출하여 방에 참가한 멤버임을 확인하고 있습니다. 방 타입이 늘어날 가능성을 고려하여, enum 분기 처리를 좀 더 확장성 있게 만들 수 있는지 확인해 보세요(예: switch-case).
195-198
: 이미 JOINED 된 Mate 검증 로직
validateRoomMember
는 참여중인지 확인하는 핵심 로직으로 보입니다. 예외 발생 시 반환되는_NOT_ROOM_MATE
가 모호하다면, 사용자나 클라이언트가 더 직관적으로 이해할 수 있는 메시지를 사용해도 좋겠습니다.
293-341
: processRoomDetails - 단일 메서드에서의 로직 집중도
해당 메서드가 방 상세 정보를 종합적으로 구성합니다. 방장 탐색, 매칭률 계산, 해시태그 추출 등 역할이 많아 복잡도가 높아질 수 있습니다. 필요하다면 세부 단계를 메서드로 재분리하거나, 도메인 레벨에서 DTO 생성을 분담하는 구조를 고려해 보세요.src/main/java/com/cozymate/cozymate_server/domain/mate/repository/MateRepository.java (1)
29-29
: RoomId와 isRoomManager 기반 조회 시 Eager 로딩
@EntityGraph(attributePaths = {"member", "member.memberStat"})
를 통해 방장 정보를 즉시 로드합니다. 방장만 조회할 때 불필요한 잘못된 로딩이 발생할 가능성을 검토해 보시는 것이 좋습니다.src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomRecommendService.java (1)
75-82
: N+1 문제 해결을 위한 효율적인 구현입니다.단일 쿼리로 여러 방의 해시태그를 조회하도록 개선되었습니다. Stream API를 활용한 데이터 변환도 깔끔하게 구현되었습니다.
다만, 성능 최적화를 위해 다음과 같은 개선을 고려해보세요:
- roomList.stream().map(Room::getId).toList() + roomList.stream() + .map(Room::getId) + .distinct() // 중복 ID 제거 + .toList()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/com/cozymate/cozymate_server/domain/mate/repository/MateRepository.java
(2 hunks)src/main/java/com/cozymate/cozymate_server/domain/room/Room.java
(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/room/controller/RoomController.java
(2 hunks)src/main/java/com/cozymate/cozymate_server/domain/room/converter/RoomConverter.java
(2 hunks)src/main/java/com/cozymate/cozymate_server/domain/room/converter/RoomRecommendConverter.java
(2 hunks)src/main/java/com/cozymate/cozymate_server/domain/room/repository/RoomRepository.java
(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomQueryService.java
(8 hunks)src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomRecommendService.java
(5 hunks)src/main/java/com/cozymate/cozymate_server/domain/roomfavorite/repository/RoomFavoriteRepository.java
(1 hunks)src/main/java/com/cozymate/cozymate_server/domain/roomfavorite/service/RoomFavoriteQueryService.java
(3 hunks)src/main/java/com/cozymate/cozymate_server/domain/roomhashtag/RoomHashtag.java
(2 hunks)src/main/java/com/cozymate/cozymate_server/domain/roomhashtag/repository/RoomHashtagRepository.java
(1 hunks)
🔇 Additional comments (30)
src/main/java/com/cozymate/cozymate_server/domain/room/repository/RoomRepository.java (1)
28-31
: 쿼리 최적화가 잘 되었습니다!서브쿼리와 DISTINCT를 사용하여 중복을 제거하고 단일 쿼리로 데이터를 조회하는 방식이 효율적입니다.
src/main/java/com/cozymate/cozymate_server/domain/room/converter/RoomConverter.java (1)
70-72
: DTO 변환 로직이 깔끔합니다!초대된 방 목록을 응답 DTO로 변환하는 로직이 명확하고 간단하게 구현되었습니다.
src/main/java/com/cozymate/cozymate_server/domain/room/Room.java (1)
1-122
:❓ Verification inconclusive
roomHashtags 관계 제거에 따른 영향도 확인이 필요합니다.
roomHashtags 필드가 제거되어 n+1 문제는 해결되었으나, 이 변경으로 인한 영향을 받는 다른 코드들이 모두 적절히 수정되었는지 확인이 필요합니다.
🏁 Script executed:
#!/bin/bash # roomHashtags 필드를 참조하는 코드 검색 rg "roomHashtags" -lLength of output: 381
roomHashtags 필드 제거와 관련한 연관 코드 검증 필요
roomHashtags 필드가 제거되어 n+1 문제 해결은 확인되었으나, 아래 파일들에서 여전히 roomHashtags를 참조하고 있습니다. 해당 파일들이 변경 사항을 반영하도록 업데이트 되었는지 재확인해 주세요:
src/main/java/com/cozymate/cozymate_server/domain/room/repository/RoomRepository.java
src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomQueryService.java
src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomRecommendService.java
src/main/java/com/cozymate/cozymate_server/domain/roomfavorite/service/RoomFavoriteQueryService.java
src/main/java/com/cozymate/cozymate_server/domain/roomfavorite/repository/RoomFavoriteRepository.java (1)
30-30
: 성능 개선이 잘 이루어졌습니다!엔티티 전체를 로드하는 대신 ID를 사용하여 조회하도록 변경한 것이 성능 측면에서 효율적입니다.
src/main/java/com/cozymate/cozymate_server/domain/roomhashtag/RoomHashtag.java (1)
30-36
: JPA 관계 설정이 최적화되었습니다!
FetchType.LAZY
를 사용하여 N+1 문제를 방지했습니다.@JoinColumn
어노테이션으로 외래 키 관계를 명확하게 정의했습니다.src/main/java/com/cozymate/cozymate_server/domain/roomhashtag/repository/RoomHashtagRepository.java (1)
16-18
: 쿼리 최적화가 잘 되었습니다!
@EntityGraph
를 사용하여 hashtag를 효율적으로 조회합니다.- IN 절을 사용하여 여러 room의 해시태그를 한 번에 조회하도록 최적화했습니다.
src/main/java/com/cozymate/cozymate_server/domain/room/controller/RoomController.java (1)
112-112
: API 문서화와 매개변수 전달이 개선되었습니다!
- API 설명이 더 명확해졌습니다.
- Member 객체를 직접 전달하여 코드가 더 명확해졌습니다.
Also applies to: 379-379
src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomQueryService.java (14)
50-52
: 메서드 체이닝 구조가 명확함
validateRoom(roomId)
→validateRoomAccess(room, memberId)
→processRoomDetails(...)
순서가 직관적이라 가독성이 좋습니다. 단,validateRoomAccess
에서 발생 가능한 예외상황(예: PRIVATE 방에 대한 권한 검증)과processRoomDetails
의 예외처리를 명확히 구분 관리하는지도 확인해 주세요.
58-58
: 중복 로직 간소화
getRoomByInviteCode
의 반환부에서processRoomDetails
를 바로 호출하는 로직은 중복 검증 로직을 줄이는 점에서 효율적입니다.
90-93
: getExistRoom 활용과 권한 검증 흐름 주의
validateRoom(getExistRoom(managerId).roomId())
와validateRoomManager(room.getId(), managerId)
를 연쇄적으로 호출해 매니저 권한을 검증하고 있습니다. 다만getExistRoom
의 결과가 null을 반환하는 경우도 고려해야 하므로, 내부에서 예외 처리 흐름이 안전하게 구성되어 있는지 확인해 주세요.
117-125
: 방 목록 조회 시 참조 객체 맵 구성 방식
findRoomsWithMates
→getRoomHashtagsMap
→findRoomManagers
순으로 데이터를 맵핑하여 필요한 정보를 모읍니다. 이러한 방식은 성능상 잦은 DB 접근을 줄이는 데에 탁월합니다. 단, 각 Map 인셉션 시 Key 충돌 등 에러Handling이 없는지 확인이 필요합니다.
136-137
: 해시태그 추출 로직의 단일 책임 원칙
List<String> hashtags = extractHashtags(room, roomHashtagsMap);
는 단순 해시태그 추출 로직으로 간소화되어 있습니다. 이처럼 분리된 상태를 유지하면, 추후 해시태그 필터링 또는 전처리가 필요해질 때 쉽게 확장할 수 있을 것입니다.
147-148
: managerMate가 null인 경우 예외 상황 처리
managerMate
에서getMember()
를 바로 호출하기 때문에, 매니저가 존재하지 않을 경우(또는 삭제된 경우) NullPointerException 발생 가능성이 있습니다. 상위 로직에서 이미 방장 존재를 보장한다면 괜찮지만, 추가 안전 장치가 필요하다면 한 번 더 null 체크를 고려해주세요.Also applies to: 153-153
177-180
: 기숙사 이름 가져오기 로직
getDormitoryName(managerMate)
가 단일 책임으로 잘 분리되어 있습니다. null-safe 처리(orElse("")
)가 되어 있어 예외 발생 가능성이 낮은 점이 좋습니다.
200-206
: RoomManager 검증 시 예외 케이스
방장 정보를 찾을 수 없을 때_ROOM_MANAGER_NOT_FOUND
예외를 던지고, 일치하지 않을 때_NOT_ROOM_MANAGER
예외를 던집니다. 논리적으로 일관성 있어 보이지만, 방장이 없거나 데이터가 무결하지 않은 상황에서의 예외처리가 확실히 필요한지(혹은 DB 제약조건으로 방장 반드시 1명 존재를 보장할지) 다시 한 번 고려해 주세요.
209-209
: 검색 메서드 파라미터 의도
public List<RoomSearchResponseDTO> searchRooms(String keyword, Member member)
형태로 변경되어 있어, 검색 범위를 멤버 정보에 따라 다르게 필터링하는 것으로 보입니다. 구현 의도대로 각 필드(gender, universityId 등)가 제대로 반영되는지 테스트 케이스를 통해 검증해 주세요.
230-230
: equality 정렬 로직 확인
Comparator.nullsLast(Comparator.reverseOrder())
로 정렬 시, 일치율(null)이 가장 뒤로 오게 설계되어 있습니다. 실제 상황에서 null이 발생할 수 있는지, 그리고 null일 경우 UI·UX 면에서 어떻게 처리할지 확인이 필요합니다.
343-350
: calculateLifestyleMatchRates 메서드
라이프스타일 매칭률을 계산하여 Map을 반환하는 구조가 명확합니다. 추가적으로, 반환 Map이 사용되는 범위가 한정적이라면 캐싱이나 재사용 여부도 검토해볼 수 있습니다.
352-360
: getRoomHashtagsMap에서의 groupBy 처리
룸 해시태그 목록을 roomId 기준으로 그룹화하여 반환하는 구조가 깔끔합니다. 다만 room이 없는 경우(빈 리스트)에도 안전하게 처리되도록 신경 써 주세요.
362-364
: extractHashtags 단순함 유지
getOrDefault(room.getId(), List.of())
로 NPE 위험성이 없고, 해시태그가 없는 경우 빈 리스트를 반환합니다. 불필요한 로직이나 사이드 이펙트가 없어 유지보수하기 수월합니다.
366-369
: validateRoom의 예외 처리
roomRepository.findById(roomId)
가 없을 경우_ROOM_NOT_FOUND
예외를 던집니다. NotFound 상황에 대한 예외 처리가 명확하므로, 해당 예외가 상위 레벨에서 어떻게 처리(예: 404 응답)되는지도 점검해 주세요.src/main/java/com/cozymate/cozymate_server/domain/room/converter/RoomRecommendConverter.java (2)
12-16
: 해시태그 파라미터 추가로 인한 메서드 시그니처 확장
toRoomRecommendationResponse
메서드가hashtags
파라미터를 직접 받아 DTO에 세팅하는 방식으로 변경되었습니다. 해시태그를 기존에 내부에서 추출하던 로직 제거로, 책임 분산이 명확해지고 테스트도 용이해졌습니다.
25-29
: toRoomRecommendationResponseWhenNoMemberStat의 일관성
멤버 통계를 사용하지 않는 추천 로직에서도 동일하게hashtags
가 인자로 전달됩니다. 통일된 인터페이스로 향후 기능 확장 및 코드 가독성이 향상되었습니다.src/main/java/com/cozymate/cozymate_server/domain/mate/repository/MateRepository.java (5)
11-11
: EntityGraph 사용으로 인한 Eager 로딩 유의
import org.springframework.data.jpa.repository.EntityGraph;
로 Eager 로딩을 설정하면, 쿼리 수는 줄어들지만 대량의 데이터를 한 번에 가져올 수 있습니다. Mate와 MemberStat 간 조인 시 필요 이상의 데이터까지 로드되지 않는지 확인이 필요합니다.
19-21
: JOINED 상태의 Mate를 조인하여 가져오는 전용 메서드
findJoinedMatesWithMemberAndStats
는 N+1 문제를 해결하기 위한 좋은 접근입니다. 다만, 동시에 너무 많은 필드를 한꺼번에 가져오지 않는지(불필요한 칼럼 읽기 부담) 주의해주세요.
23-25
: isRoomManager가 true인 Mate 조회 메서드
findRoomManagers
는 여러 Room ID에 대해 방장 정보를 효율적으로 조회합니다. 이로 인해 RoomQueryService 등에서 한 번의 쿼리로 방장 정보를 맵핑할 수 있어 성능 향상에 기여합니다.
27-27
: findByRoomId 시 사용 의도
List<Mate> findByRoomId(Long roomId);
를 단순 조회용으로 사용하는 경우가 많다면, 불필요한 테이블 조인 없이 가볍게 접근할 수 있어 좋은 선택입니다. 단, 필요에 따라 MemberStat 등을 fetch해야 한다면 별도의 메서드를 사용하는 게 낫습니다.
56-56
: findAllByRoomIdAndEntryStatus의 Eager 로딩 주의
findAllByRoomIdAndEntryStatus
에도 Eager 로딩이 설정되었습니다. 특정 경우에는 JOIN이 너무 깊어지면 성능 병목이 생길 수 있으니, 쿼리 실행 빈도와 결과 집합 크기를 지속적으로 모니터링하는 것을 권장합니다.src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomRecommendService.java (2)
13-13
: 의존성 주입이 적절하게 구현되었습니다.RoomHashtagRepository가 적절하게 주입되어 있으며, 필요한 import 문도 올바르게 추가되었습니다.
Also applies to: 20-20, 45-45
172-174
: Null 안전성이 잘 처리되었습니다.
getOrDefault
를 사용하여 해시태그가 없는 경우도 안전하게 처리하고 있습니다.
// 방 해시태그 조회 | ||
Map<Long, List<String>> roomHashtagsMap = roomHashtagRepository.findByRoomIds( | ||
roomList.stream().map(Room::getId).toList()) | ||
.stream() | ||
.collect(Collectors.groupingBy( | ||
roomHashtag -> roomHashtag.getRoom().getId(), | ||
Collectors.mapping(roomHashtag -> roomHashtag.getHashtag().getHashtag(), Collectors.toList()) | ||
)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
코드 중복이 발견되었습니다.
해시태그 조회 로직이 getRecommendationList
메소드와 동일하게 중복되어 있습니다.
다음과 같이 공통 메소드로 추출하는 것을 추천드립니다:
+ private Map<Long, List<String>> fetchRoomHashtags(List<Room> roomList) {
+ return roomHashtagRepository.findByRoomIds(
+ roomList.stream().map(Room::getId).distinct().toList())
+ .stream()
+ .collect(Collectors.groupingBy(
+ roomHashtag -> roomHashtag.getRoom().getId(),
+ Collectors.mapping(roomHashtag -> roomHashtag.getHashtag().getHashtag(), Collectors.toList())
+ ));
+ }
그리고 두 위치에서 다음과 같이 사용:
- Map<Long, List<String>> roomHashtagsMap = roomHashtagRepository.findByRoomIds(...
+ Map<Long, List<String>> roomHashtagsMap = fetchRoomHashtags(roomList);
📝 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.
// 방 해시태그 조회 | |
Map<Long, List<String>> roomHashtagsMap = roomHashtagRepository.findByRoomIds( | |
roomList.stream().map(Room::getId).toList()) | |
.stream() | |
.collect(Collectors.groupingBy( | |
roomHashtag -> roomHashtag.getRoom().getId(), | |
Collectors.mapping(roomHashtag -> roomHashtag.getHashtag().getHashtag(), Collectors.toList()) | |
)); | |
// 기존 코드에서 방 해시태그 조회 부분을 다음과 같이 변경 | |
// 방 해시태그 조회 | |
Map<Long, List<String>> roomHashtagsMap = fetchRoomHashtags(roomList); | |
// ... (other class code) | |
// 새로 추출된 공통 메소드 추가 | |
private Map<Long, List<String>> fetchRoomHashtags(List<Room> roomList) { | |
return roomHashtagRepository.findByRoomIds( | |
roomList.stream().map(Room::getId).distinct().toList()) | |
.stream() | |
.collect(Collectors.groupingBy( | |
roomHashtag -> roomHashtag.getRoom().getId(), | |
Collectors.mapping(roomHashtag -> roomHashtag.getHashtag().getHashtag(), Collectors.toList()) | |
)); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeRabbit의 말처럼 RoomHashtag를 조회해서 그룹화하는 코드가 여러번 중복되는 것 같아보이는데, RoomHashTag쪽의 QueryService나 RepositoryService 내에서 해당 중복코드를 메서드화해서 관리하면 좋을 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for RoomHashtagQueryService or RoomHashtagRepositoryService
rg -l "RoomHashtagQueryService|RoomHashtagRepositoryService" --type java
# Also check for any service classes in the roomhashtag package
fd ".*Service\.java" src/main/java/com/cozymate/cozymate_server/domain/roomhashtag/
# Check the structure of the roomhashtag package
find src/main/java/com/cozymate/cozymate_server/domain/roomhashtag/ -type d
Length of output: 617
🏁 Script executed:
#!/bin/bash
# Let's check the RoomHashtagCommandService to see its structure
cat src/main/java/com/cozymate/cozymate_server/domain/roomhashtag/service/RoomHashtagCommandService.java
# Also check the RoomHashtagRepository
cat src/main/java/com/cozymate/cozymate_server/domain/roomhashtag/repository/RoomHashtagRepository.java
# Let's find other places where the hashtag grouping code is duplicated
rg -A 5 "roomHashtagRepository.findByRoomIds" --type java
Length of output: 6895
@eple0329 좋은 의견 감사합니다! 코드베이스를 확인해보니 RoomHashtag
관련 코드가 여러 서비스에서 중복되고 있네요. Command/Query 패턴을 사용하고 있는 것 같아 별도의 RoomHashtagQueryService
를 만드는 것이 좋을 것 같습니다. 아래와 같이 구현할 수 있습니다:
package com.cozymate.cozymate_server.domain.roomhashtag.service;
import com.cozymate.cozymate_server.domain.room.Room;
import com.cozymate.cozymate_server.domain.roomhashtag.repository.RoomHashtagRepository;
import jakarta.transaction.Transactional;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
@Service
@RequiredArgsConstructor
@Transactional
public class RoomHashtagQueryService {
private final RoomHashtagRepository roomHashtagRepository;
/**
* 룸 아이디 리스트로 해시태그를 조회하여 룸 ID별로 그룹화하여 반환합니다.
*
* @param roomIds 룸 아이디 리스트
* @return 룸 ID를 키로, 해시태그 문자열 리스트를 값으로 하는 맵
*/
public Map<Long, List<String>> getRoomHashtagsGroupedByRoomId(List<Long> roomIds) {
return roomHashtagRepository.findByRoomIds(roomIds)
.stream()
.collect(Collectors.groupingBy(
roomHashtag -> roomHashtag.getRoom().getId(),
Collectors.mapping(roomHashtag -> roomHashtag.getHashtag().getHashtag(), Collectors.toList())
));
}
/**
* 룸 리스트로 해시태그를 조회하여 룸 ID별로 그룹화하여 반환합니다.
*
* @param rooms 룸 리스트
* @return 룸 ID를 키로, 해시태그 문자열 리스트를 값으로 하는 맵
*/
public Map<Long, List<String>> getRoomHashtagsGroupedByRoomId(List<Room> rooms) {
List<Long> roomIds = rooms.stream().map(Room::getId).distinct().toList();
return getRoomHashtagsGroupedByRoomId(roomIds);
}
}
이렇게 하면 RoomRecommendService
와 다른 서비스들에서는 다음과 같이 간단하게 사용할 수 있습니다:
// 기존 코드
Map<Long, List<String>> roomHashtagsMap = roomHashtagRepository.findByRoomIds(
roomList.stream().map(Room::getId).toList())
.stream()
.collect(Collectors.groupingBy(
roomHashtag -> roomHashtag.getRoom().getId(),
Collectors.mapping(roomHashtag -> roomHashtag.getHashtag().getHashtag(), Collectors.toList())
));
// 새로운 코드
Map<Long, List<String>> roomHashtagsMap = roomHashtagQueryService.getRoomHashtagsGroupedByRoomId(roomList);
이렇게 하면 중복 코드가 제거되고 변경 사항이 생겼을 때 한 곳만 수정하면 되어 유지보수가 더 쉬워집니다.
✏️ Learnings added
Learnt from: eple0329
PR: cozy-mate/cozymate-Server#281
File: src/main/java/com/cozymate/cozymate_server/domain/room/service/RoomRecommendService.java:223-230
Timestamp: 2025-02-25T04:58:16.864Z
Learning: 해시태그 조회와 그룹화 로직은 RoomHashtagQueryService의 getRoomHashtagsGroupedByRoomId 메서드로 추출하여 관리하는 것이 좋습니다. 이렇게 하면 중복 코드가 제거되고 변경 사항이 있을 때 한 곳만 수정하면 되어 유지보수가 수월해집니다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "hashtag_id") | ||
private Hashtag hashtag; | ||
|
||
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "room_id") | ||
private Room room; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RoomHashtag에서 Room과 Hashtag 관계를 관리하도록 수정했습니다
Map<Long, List<String>> roomHashtagsMap = roomHashtagRepository.findByRoomIds(responseRoomList.stream().map(Room::getId).toList()) | ||
.stream() | ||
.collect(Collectors.groupingBy( | ||
roomHashtag -> roomHashtag.getRoom().getId(), | ||
Collectors.mapping(roomHashtag -> roomHashtag.getHashtag().getHashtag(), Collectors.toList()) | ||
)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
map으로 room들에 대한 hashtag정보를 저장하도록 하였습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분 stream 밖으로 빼야할 거 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그리고 roomHashtag.getHashtag().getHashtag() 뭔가 좀 이상한데 Hashtag 엔티티의 hashtag 필드명 다른걸로 바꾸는건 어떨가요? 그냥 name 정도로 해도 될 것 같습니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다~
방 데이터 삭제 로직만 한번 점검 부탁드려용
@@ -68,9 +65,6 @@ public class Room extends BaseTimeEntity { | |||
@Enumerated(EnumType.STRING) | |||
private RoomType roomType; | |||
|
|||
@OneToMany(fetch = FetchType.LAZY, mappedBy = "room", cascade = CascadeType.ALL, orphanRemoval = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cascade가 사라졌으니 방 삭제 로직에서 해당 연관된 Hashtag 삭제하고 Room 하는 과정이 있어야 할 것 같네요. (사실 삭제로직을 보지는 않았고, 그냥 변경사항만 봤을 때 이야기입니다.)
@@ -10,4 +13,8 @@ public interface RoomHashtagRepository extends JpaRepository<RoomHashtag, Long> | |||
@Query("DELETE FROM RoomHashtag rh WHERE rh.room.id = :roomId") | |||
void deleteAllByRoomId(Long roomId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
삭제과정에 있는줄 알았는데, RoomUpdate 부분에만 있네용
deleteRoomDatas 메서드 내에서도 해당 삭제 로직이 있어야할 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm~ 코멘트 몇가지 남겨뒀습니다
Map<Long, List<String>> roomHashtagsMap = roomHashtagRepository.findByRoomIds(responseRoomList.stream().map(Room::getId).toList()) | ||
.stream() | ||
.collect(Collectors.groupingBy( | ||
roomHashtag -> roomHashtag.getRoom().getId(), | ||
Collectors.mapping(roomHashtag -> roomHashtag.getHashtag().getHashtag(), Collectors.toList()) | ||
)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분 stream 밖으로 빼야할 거 같아요
Map<Long, List<String>> roomHashtagsMap = roomHashtagRepository.findByRoomIds(responseRoomList.stream().map(Room::getId).toList()) | ||
.stream() | ||
.collect(Collectors.groupingBy( | ||
roomHashtag -> roomHashtag.getRoom().getId(), | ||
Collectors.mapping(roomHashtag -> roomHashtag.getHashtag().getHashtag(), Collectors.toList()) | ||
)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그리고 roomHashtag.getHashtag().getHashtag() 뭔가 좀 이상한데 Hashtag 엔티티의 hashtag 필드명 다른걸로 바꾸는건 어떨가요? 그냥 name 정도로 해도 될 것 같습니다~
@@ -1,6 +1,9 @@ | |||
package com.cozymate.cozymate_server.domain.roomhashtag.repository; | |||
|
|||
import com.cozymate.cozymate_server.domain.roomhashtag.RoomHashtag; | |||
import io.lettuce.core.dynamic.annotation.Param; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import org.springframework.data.repository.query.Param; 이걸로 수정해야할 것 같네여
얘는 아마 redis꺼 같습니다
⚒️develop의 최신 커밋을 pull 받았나요?
네
#️⃣ 작업 내용
RoomHashtag의 관계를 수정하여 n+1문제를 해결했습니다
수정하면서 RoomRecommand, RoomFavorite 코드도 살짝 변경되어서 확인 부탁드립니다
동작 확인
방 여러개 조회시에도 한번만 조회됩니다
💬 리뷰 요구사항(선택)
Summary by CodeRabbit
New Features
Refactor
Chores