Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughV2 그룹 상세 조회 기능 구현을 위해 공통 DTO(주소/작성자/이미지), 응답 DTO 마이그레이션, 레포지토리 쿼리 추가, 서비스 getGroup 메서드와 컨트롤러 GET 엔드포인트 및 통합 HTTP 테스트가 추가됨. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as GroupV2Controller
participant Service as GroupV2Service
participant GroupRepo as GroupV2Repository
participant ImgRepo as GroupImageV2Repository
participant UserRepo as GroupUserV2Repository
participant DB as Database
Client->>Controller: GET /api/v2/groups/{groupId} (Auth optional)
Controller->>Service: getGroup(userIdOrNull, groupId)
Service->>GroupRepo: findGroupWithHostAndTags(groupId)
GroupRepo->>DB: fetch group + host + tags
DB-->>GroupRepo: GroupV2
GroupRepo-->>Service: Optional<GroupV2>
par Parallel fetch images & users
Service->>ImgRepo: findAllByGroupIdWithVariants(groupId)
ImgRepo->>DB: fetch images + variants
DB-->>ImgRepo: List<GroupImageV2>
ImgRepo-->>Service: images
Service->>UserRepo: findAllByGroupIdWithUser(groupId)
UserRepo->>DB: fetch group users + user
DB-->>UserRepo: List<GroupUserV2>
UserRepo-->>Service: users
end
Note over Service: Transform domain → DTOs\n(Address.from, CreatedBy.from,\nGroupImageItem.from, MyMembership.from, JoinedMember.from)
Service-->>Controller: GetGroupV2Response
Controller-->>Client: ApiResponse<GetGroupV2Response> (200 OK)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements V2 group detail retrieval functionality using a three-query fetch join strategy to avoid MultipleBagFetchException and result row explosion issues. The implementation separates collection fetches into distinct queries for group+host+tags, images+variants, and users.
Key changes:
- Added
getGroupendpoint to retrieve detailed group information including user participation status - Refactored common DTOs (Address, CreatedBy, GroupImageItem) into a shared package for reuse across responses
- Implemented three separate repository queries with fetch joins to efficiently load related entities
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
GroupV2Controller.java |
Added GET endpoint for group detail retrieval with authentication support |
GroupV2Service.java |
Implemented getGroup method with three separate fetch join queries |
GroupV2Repository.java |
Added query to fetch group with host and tags using fetch join |
GroupImageV2Repository.java |
New repository with query to fetch group images with variants |
GroupUserV2Repository.java |
New repository with query to fetch group users with user details |
GetGroupV2Response.java |
Refactored response DTO with MyMembership and JoinedMember records |
CreateGroupV2Response.java |
Removed duplicate DTO definitions, now uses common DTOs |
Address.java |
Extracted common Address DTO |
CreatedBy.java |
Extracted common CreatedBy DTO |
GroupImageItem.java |
Extracted common GroupImageItem DTO |
GroupImageVariantItem.java |
Extracted common GroupImageVariantItem DTO |
v2-group-get.http |
New HTTP test file for group detail retrieval endpoint |
v2-group-create.http |
New HTTP test file for V2 group creation scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| %} | ||
|
|
||
| ### V2 로그인 후, 현재 유저 기준 참여 상태 포함하여 상세 조회 | ||
| GET http://localhost:8080/api/v2/groups/1 |
There was a problem hiding this comment.
The hardcoded group ID '1' may not correspond to the group created in the previous step. The script saves the created group ID to the variable 'groupId' (line 90), but the GET request uses the hardcoded value '1' instead of '{{groupId}}'. This will likely result in a 404 error if the created group has a different ID.
| .orElseThrow( | ||
| () -> new GroupException(GroupErrorCode.GROUP_NOT_FOUND_BY_ID, groupId)); | ||
|
|
||
| // 컬렉션은 안전하게 따로 보관해서 옮겨야 좋다고 한다. |
There was a problem hiding this comment.
The comment "컬렉션은 안전하게 따로 보관해서 옮겨야 좋다고 한다" (Collections should be safely stored separately) is vague and doesn't clearly explain why this pattern is being used. Consider updating the comment to explicitly state this avoids MultipleBagFetchException and N+1 issues, which aligns with the PR description's reasoning.
| List<GroupImageV2> images = groupImageV2Repository.findAllByGroupIdWithVariants(groupId); | ||
| List<GroupUserV2> users = groupUserV2Repository.findAllByGroupIdWithUser(groupId); | ||
|
|
||
| return GetGroupV2Response.of(group, images, users, userDetails.getId()); |
There was a problem hiding this comment.
The method calls userDetails.getId() without null checking. If the endpoint is accessed without authentication (when userDetails is null), this will cause a NullPointerException. According to the response design where myMembership can be null for unauthenticated users, the controller should allow unauthenticated access. Consider passing userDetails directly to GetGroupV2Response.of() and handle the null case there, or add proper null checking here.
| @@ -0,0 +1,10 @@ | |||
| package team.wego.wegobackend.group.v2.application.dto.common; | |||
|
|
|||
| import team.wego.wegobackend.group.v2.application.dto.response.GetGroupV2Response; | |||
There was a problem hiding this comment.
The import statement references GetGroupV2Response which is not used in this file. This appears to be an unnecessary import that should be removed.
| gu.getStatus() == GroupUserV2Status.ATTEND, | ||
| gu.getId(), | ||
| gu.getGroupRole(), | ||
| gu.getStatus(), | ||
| gu.getJoinedAt(), | ||
| gu.getLeftAt() |
There was a problem hiding this comment.
The logic for determining isJoined only checks if status equals ATTEND, but this doesn't match all other status fields being populated. When a user has LEFT status (status != ATTEND), isJoined is set to false but other fields like groupUserId, role, status, joinedAt, and leftAt are still populated with actual values. This creates an inconsistency where isJoined=false but the membership record still contains data. Consider whether isJoined should be true for any status where a GroupUserV2 record exists, or ensure all fields are null when isJoined is false.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/main/java/team/wego/wegobackend/group/v2/application/dto/common/Address.java (1)
3-3: 사용하지 않는 import 제거가 필요합니다.
GetGroupV2Responseimport가 이 파일에서 사용되지 않습니다.다음 diff를 적용하여 사용하지 않는 import를 제거하세요:
-import team.wego.wegobackend.group.v2.application.dto.response.GetGroupV2Response; import team.wego.wegobackend.group.v2.domain.entity.GroupV2Address;src/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupV2Repository.java (1)
12-20: 쿼리 구조가 올바릅니다. 조인 최적화 검토를 권장합니다.distinct와 fetch join 조합이 올바르게 사용되어 N+1 문제를 방지합니다. Line 17의
left join fetch gt.tag는GroupTagV2가Tag를 필수로 가지는 경우 일반join fetch로 변경할 수 있습니다.
GroupTagV2엔티티에서tag관계가optional=false인지 확인하세요:#!/bin/bash # Description: GroupTagV2의 tag 필드가 optional인지 확인 # GroupTagV2 엔티티에서 tag 필드의 @ManyToOne 어노테이션 확인 rg -nP --type=java -A2 '@ManyToOne.*\n.*tag' --glob='**/GroupTagV2.java'만약
optional=false라면 다음과 같이 변경을 고려하세요:- left join fetch gt.tag t + join fetch gt.tag tsrc/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java (1)
165-165: 주석을 더 명확하게 작성하는 것을 권장합니다.Line 165의 주석이 설계 의도를 설명하고 있으나, "~고 한다"는 표현이 불확실함을 암시합니다. MultipleBagFetchException 회피라는 기술적 이유를 명시하면 더 명확합니다.
다음과 같이 주석을 개선할 수 있습니다:
- // 컬렉션은 안전하게 따로 보관해서 옮겨야 좋다고 한다. + // MultipleBagFetchException 방지를 위해 컬렉션을 별도 쿼리로 조회 List<GroupImageV2> images = groupImageV2Repository.findAllByGroupIdWithVariants(groupId); List<GroupUserV2> users = groupUserV2Repository.findAllByGroupIdWithUser(groupId);src/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupUserV2Repository.java (1)
11-17: 쿼리 구현이 올바릅니다.
join fetch를 사용하여User엔티티를 즉시 로딩하여 N+1 문제를 방지합니다. 쿼리 로직이 정확합니다.선택사항: 조회 결과의 일관성을 위해
order by절 추가를 고려하세요(예:order by gu.joinedAt asc또는order by gu.groupRole, gu.joinedAt).@Query(""" select gu from GroupUserV2 gu join fetch gu.user u where gu.group.id = :groupId + order by gu.joinedAt asc """)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/team/wego/wegobackend/group/v2/application/dto/common/Address.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/common/CreatedBy.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/common/GroupImageItem.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/common/GroupImageVariantItem.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/response/CreateGroupV2Response.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupV2Response.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java(3 hunks)src/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupImageV2Repository.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupUserV2Repository.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupV2Repository.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/presentation/GroupV2Controller.java(3 hunks)src/test/http/group/v2/v2-group-get.http(1 hunks)
🔇 Additional comments (10)
src/main/java/team/wego/wegobackend/group/v2/presentation/GroupV2Controller.java (1)
44-52: 구현이 올바릅니다.새로운 GET 엔드포인트가 깔끔하게 구현되었으며 기존 패턴을 잘 따르고 있습니다. 인증 처리와 응답 구조가 적절합니다.
src/main/java/team/wego/wegobackend/group/v2/application/dto/common/GroupImageVariantItem.java (1)
7-26: 구현이 올바릅니다.레코드와 팩토리 메서드가 올바르게 구현되어 있습니다.
GroupImageV2VariantTypeenum에서 width/height를 가져오는 설계가 좋습니다.src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java (2)
38-39: 의존성 추가가 올바릅니다.새로운 저장소 의존성이 올바르게 선언되었습니다.
158-170: 저장소 메서드가 모두 올바르게 구현되어 있습니다.모든 필요한 저장소 메서드가 확인되었습니다:
GroupV2Repository.findGroupWithHostAndTags()✓GroupImageV2Repository.findAllByGroupIdWithVariants()✓GroupUserV2Repository.findAllByGroupIdWithUser()✓읽기 전용 트랜잭션과 3개의 분리된 fetch join 쿼리로 MultipleBagFetchException을 올바르게 회피했으며, Line 165의 주석이 설계 의도를 잘 설명합니다.
src/main/java/team/wego/wegobackend/group/v2/application/dto/common/GroupImageItem.java (1)
6-18: 구현이 올바르고 null 안전합니다.레코드와 팩토리 메서드가 깔끔하게 구현되어 있습니다.
GroupImageV2.variants필드는new ArrayList<>()로 초기화되므로getVariants()는 절대 null을 반환하지 않습니다. 따라서 stream() 호출과 toList() 변환은 안전하며, 빈 리스트 또한 올바르게 처리됩니다.src/main/java/team/wego/wegobackend/group/v2/application/dto/response/CreateGroupV2Response.java (2)
5-6: 공통 DTO 추출 리팩토링을 승인합니다.
CreatedBy와GroupImageItem을 공통 패키지로 추출하여 코드 중복을 제거하고 재사용성을 높였습니다. 이는GetGroupV2Response와의 일관성을 유지하는 좋은 접근입니다.
62-66: 공통 DTO 매퍼 사용이 올바릅니다.공통 패키지의
CreatedBy.from()과GroupImageItem.from()메서드를 정확하게 사용하고 있습니다.src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupV2Response.java (2)
117-127:JoinedMember.from()매핑이 정확합니다.
GroupUserV2에서JoinedMember로의 변환 로직이 올바르게 구현되었습니다.
37-63: V2 엔티티 마이그레이션 로직을 승인합니다.
GroupV2,GroupImageV2,GroupUserV2를 사용한 새로운 매핑 로직이 올바르게 구현되었습니다. 태그 추출, 참여자 수 계산, 이미지 변환 로직이 모두 정확합니다.
GroupV2Repository의findGroupWithHostAndTags()쿼리에서 3개의 페치 조인 패턴이 이미 완성되어 있습니다:
join fetch g.host hleft join fetch g.groupTags gtleft join fetch gt.tag tPR 목표가 완벽하게 달성되었습니다.
src/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupImageV2Repository.java (1)
11-18: Hibernate 6.x 사용으로 인해 본 우려사항은 적용되지 않습니다.프로젝트의 Spring Boot 3.5.8은 Hibernate 6.x를 포함하고 있으며, Hibernate 6.x에서는 컬렉션 페치 조인 시 부모 엔티티 중복이 자동으로 제거됩니다. 따라서 현재 쿼리의
distinct키워드는 필수적이지 않지만, 사용해도 성능상 문제가 없습니다.
| public record Address(String location, String locationDetail) { | ||
| public static Address from(GroupV2Address address) { | ||
| return new Address(address.getLocation(), address.getLocationDetail()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*.java" -type f | head -20Repository: WeGo-Together/WeGo_BackEnd
Length of output: 1667
🏁 Script executed:
rg -n --type=java 'Address\.from\s*\(' -A 3 -B 1Repository: WeGo-Together/WeGo_BackEnd
Length of output: 1429
🏁 Script executed:
find . -name "*GroupV2Address*" -type fRepository: WeGo-Together/WeGo_BackEnd
Length of output: 150
🏁 Script executed:
cat -n ./src/main/java/team/wego/wegobackend/group/v2/domain/entity/GroupV2Address.javaRepository: WeGo-Together/WeGo_BackEnd
Length of output: 1406
🏁 Script executed:
sed -n '50,80p' ./src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupV2Response.javaRepository: WeGo-Together/WeGo_BackEnd
Length of output: 1152
🏁 Script executed:
sed -n '40,65p' ./src/main/java/team/wego/wegobackend/group/v2/application/dto/response/CreateGroupV2Response.javaRepository: WeGo-Together/WeGo_BackEnd
Length of output: 1073
🏁 Script executed:
find . -name "*GroupV2.java" -path "*/domain/entity/*" -type fRepository: WeGo-Together/WeGo_BackEnd
Length of output: 143
🏁 Script executed:
cat -n ./src/main/java/team/wego/wegobackend/group/v2/domain/entity/GroupV2.java | head -100Repository: WeGo-Together/WeGo_BackEnd
Length of output: 4100
from() 메서드에 null 안전성 처리가 필요합니다.
Address.from()은 GroupV2Address 파라미터에 대한 null 검증을 수행하지 않습니다. GroupV2.address는 데이터베이스 제약에서 nullable=false가 명시되지 않았으므로, 이론적으로 null 값이 전달될 수 있습니다. 호출 시점(GetGroupV2Response, CreateGroupV2Response)에서도 null 체크가 없으므로, null 참조 시 런타임 예외가 발생합니다.
Address.from()에 null 파라미터 검증 추가 또는- 호출 시점에서 사전 null 체크 수행
권장합니다.
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/group/v2/application/dto/common/Address.java
around lines 6 to 9, Address.from(GroupV2Address) lacks null-safety and will NPE
if a null GroupV2Address is passed; add a null check and handle it (either throw
a clear IllegalArgumentException/NullPointerException with a descriptive message
or return a default/empty Address as per project policy) or alternatively ensure
all callers perform pre-checks before calling from(); implement the chosen
approach consistently and update any callers (GetGroupV2Response,
CreateGroupV2Response) to avoid possible null dereference.
There was a problem hiding this comment.
요청 받을 때 location 확인 중. locationDetail 은 null 가능
There was a problem hiding this comment.
@LimdaeIl, 요청 시점에 location을 검증하신다는 점 확인했습니다!
그렇다면 GroupV2Address 객체 자체는 null이 아니고, 내부의 locationDetail 필드만 null 가능하다는 의미로 이해했습니다. 이 경우 Address.from() 메서드는 안전하게 동작할 것으로 보입니다.
혹시 요청 검증 로직이 컨트롤러나 DTO 레이어에 구현되어 있나요? 일관성을 위해 해당 검증 로직의 위치를 확인하고 싶습니다. 또한 GroupV2Address 엔티티의 location 필드에 @NotNull 같은 제약 조건이 명시되어 있는지 확인 부탁드립니다.
| public record CreatedBy( | ||
| Long userId, | ||
| String nickName, | ||
| String profileImage, | ||
| String profileMessage | ||
| ) { | ||
|
|
||
| public static CreatedBy from(User host) { | ||
| return new CreatedBy(host.getId(), host.getNickName(), host.getProfileImage(), | ||
| host.getProfileMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find User entity class
fd -t f 'User.java' | grep -E '(model|entity|domain)' | head -10
# Search for User class definition
rg -n --type=java 'class\s+User\b' -A 20 | head -50
# Look for profileImage and profileMessage field definitions
rg -n --type=java 'profileImage|profileMessage' -B 2 -A 1Repository: WeGo-Together/WeGo_BackEnd
Length of output: 14662
from() 메서드에서 null 안전성 처리 추가
profileImage와 profileMessage 필드는 User 엔티티에서 nullable 컬럼으로 정의되어 있으므로, 레코드 생성 시 null 값이 전달될 수 있습니다. host 파라미터에 대한 null 체크도 추가하세요.
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/group/v2/application/dto/common/CreatedBy.java
around lines 5–16, the from(User host) method needs null-safety: validate host
is non-null (e.g. Objects.requireNonNull(host, "host must not be null")) and
guard nullable User fields by converting null profileImage and profileMessage to
safe defaults (e.g. use Objects.requireNonNullElse(host.getProfileImage(), "")
and Objects.requireNonNullElse(host.getProfileMessage(), "") ) before
constructing the CreatedBy record.
There was a problem hiding this comment.
@LimdaeIl, 알겠습니다. DTO에서 null 값을 허용하는 것으로 이해했습니다. 👍
src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupV2Response.java
Show resolved
Hide resolved
| ### V2 로그인 후, 현재 유저 기준 참여 상태 포함하여 상세 조회 | ||
| GET http://localhost:8080/api/v2/groups/1 | ||
| Authorization: Bearer {{hostAccessToken}} No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
하드코딩된 groupId 대신 변수를 사용하세요.
Line 90에서 groupId 변수를 설정했지만, Line 94에서는 하드코딩된 값 1을 사용하고 있습니다. 일관성과 유연성을 위해 변수를 사용하세요.
다음 diff를 적용하세요:
### V2 로그인 후, 현재 유저 기준 참여 상태 포함하여 상세 조회
-GET http://localhost:8080/api/v2/groups/1
+GET http://localhost:8080/api/v2/groups/{{groupId}}
Authorization: Bearer {{hostAccessToken}}📝 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.
| ### V2 로그인 후, 현재 유저 기준 참여 상태 포함하여 상세 조회 | |
| GET http://localhost:8080/api/v2/groups/1 | |
| Authorization: Bearer {{hostAccessToken}} | |
| ### V2 로그인 후, 현재 유저 기준 참여 상태 포함하여 상세 조회 | |
| GET http://localhost:8080/api/v2/groups/{{groupId}} | |
| Authorization: Bearer {{hostAccessToken}} |
🤖 Prompt for AI Agents
In src/test/http/group/v2/v2-group-get.http around lines 93 to 95, the request
URL is using a hardcoded groupId (1) instead of the previously defined variable;
replace the hardcoded path segment with the variable placeholder so the GET
becomes /api/v2/groups/{{groupId}} (ensure the variable defined at line 90 is
correctly named and available in the environment).
📝 Pull Request
📌 PR 종류
해당하는 항목에 체크해주세요.
✨ 변경 내용
V2 모임 상세 조회를 모임, 이미지, 모임유저에서 페치 조인 3번 발생으로 구현했습니다.
groupTags, images, users 같은 컬렉션을 한 JPQL에 다 fetch join 하면 MultipleBagFetchException이나 결과 row 폭발/중복 문제가 생기기 쉽습니다. 엔티티그래프로도 가능하지만, 페치 조인 3쿼리 분리로 구현했습니다.
이메일 조회 1 <- 뭐지?
모임에서 모임 ID 조회 1(조인1, LEFT 조인2)
모임이미지에서 모임 ID 조회 1(LEFT 조인 1)
모임유저에서 모임 ID 조회 1(조인 1)
🔍 관련 이슈
🧪 테스트
변경된 기능에 대한 테스트 범위 또는 테스트 결과를 작성해주세요.
🚨 확인해야 할 사항 (Checklist)
PR을 제출하기 전에 아래 항목들을 확인해주세요.
🙋 기타 참고 사항
리뷰어가 참고하면 좋을 만한 추가 설명이 있다면 적어주세요.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.