Feat: [FN-121] 그룹 내 멤버 조회 API 추가 #35
Conversation
WalkthroughQueryDSL JPA를 도입해 생성 소스 경로를 빌드에 연결하고(.gitignore 포함), 그룹 멤버 조회 API(GET /v1/groups/{groupId}/members)를 추가했다. DTO(FindGroupMemberResponse, GroupMemberInfo), 레포지토리 커스텀 인터페이스/구현(QueryDSL), 서비스/컨트롤러 확장 및 관련 테스트가 추가되었다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as 클라이언트
participant Controller as GroupController
participant Service as GroupService
participant GroupRepo as GroupRepository
participant UserRepo as UserProfileRepository
participant MemberRepo as GroupMemberRepository
participant MemberRepoImpl as GroupMemberRepositoryImpl
participant QF as JPAQueryFactory
Client->>Controller: GET /v1/groups/{groupId}/members
Controller->>Service: findGroupMembers(auth, groupId)
Service->>GroupRepo: existsByIdAndDeletedAtIsNull(groupId) / getGroup(groupId)
GroupRepo-->>Service: Group
Service->>UserRepo: findById(auth.userId)
UserRepo-->>Service: UserProfile
Service->>MemberRepo: validateGroupInUser(userProfile, groupId) / getGroupMember(...)
MemberRepo-->>Service: GroupMember (검증)
Service->>MemberRepoImpl: findGroupMembers(groupId)
MemberRepoImpl->>QF: QueryDSL select+join (userProfile, groupMember)
QF-->>MemberRepoImpl: List<GroupMemberInfo>
MemberRepoImpl-->>Service: List<GroupMemberInfo>
Service-->>Controller: FindGroupMemberResponse.from(list)
Controller-->>Client: 200 OK (FindGroupMemberResponse)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (18)
.gitignore (1)
8-10: QueryDSL 생성 경로(.gitignore)와 빌드 설정(build.gradle) 간 불일치 정리 제안현재 빌드가 QueryDSL 소스를
build/generated/querydsl아래로 생성하도록 설정되어 있는데(.gradle/clean도 포함),.gitignore에는/src/main/generated/만 무시하도록 추가되어 있습니다. 실사용 경로와 무관한 ignore 규칙은 오해를 유발할 수 있어 정리하는 편이 좋습니다.build/전체가 이미 무시되므로 별도 조치가 없어도 생성 파일은 커밋되지 않습니다.아래 중 하나로 정리하는 것을 권장합니다.
/src/main/generated/규칙 제거- 혹은 주석을 추가해 “현재는 build 경로를 사용함”을 명시
적용 예시(diff):
### QueryDSL ### -/src/main/generated/ +# Generated Q-types are emitted to build/generated/querydsl (see build.gradle). +# Keep the following rule only if you switch generation to src/main/generated in the future. +# /src/main/generated/src/main/java/project/flipnote/infra/config/QuerydslConfig.java (1)
11-20: JPAQueryFactory 빈 등록은 적절합니다 + 생성자 주입 간소화 제안구성 자체는 표준적이고 문제 없습니다. 사소 제안으로, 설정 클래스에서 Lombok과 필드 보관을 생략하고 “메서드 파라미터 주입”을 사용하면 의존성 표면을 더 줄일 수 있습니다. 기능은 동일합니다.
간소화 예시(diff):
package project.flipnote.infra.config; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import com.querydsl.jpa.impl.JPAQueryFactory; -import jakarta.persistence.EntityManager; -import lombok.RequiredArgsConstructor; +import jakarta.persistence.EntityManager; @Configuration -@RequiredArgsConstructor public class QuerydslConfig { - private final EntityManager entityManager; - @Bean - public JPAQueryFactory jpaQueryFactory() { - return new JPAQueryFactory(entityManager); + public JPAQueryFactory jpaQueryFactory(EntityManager entityManager) { + return new JPAQueryFactory(entityManager); } }build.gradle (3)
55-59: QueryDSL JPA 의존성 구성 적절
querydsl-jpa:5.0.0:jakarta및 APT 설정이 Spring Boot 3.x(Jakarta) 환경과 맞습니다. 현재 형태로 충분히 빌드·생성이 동작할 것으로 보입니다.
선택사항: 버전 상수를 ext(또는 version catalog)로 올려서 중앙 관리하면 추후 업그레이드가 수월합니다.
75-76: clean 훅 중복 정리 제안
build/전체를 이미 Gradle 기본clean이 제거하므로file(querydslDir).deleteDir()는 실질적으로 중복입니다. 제거해도 무방합니다.정리 예시(diff):
-clean.doLast { - file(querydslDir).deleteDir() -}
63-65: 생성 디렉터리 설정 범위 축소(compileJava 한정) 권장 및 검증 요청파일:
build.gradle(63–65줄)문제 요약
tasks.withType(JavaCompile).configureEach { … }를 사용하면compileJava뿐 아니라compileTestJava까지 동일한build/generated/querydsl디렉터리를 생성 경로로 강제 적용합니다.- 테스트 소스에서 APT가 동작할 경우(main/test 산출물이 한 폴더에 섞이는) 충돌 가능성이 있습니다.
권장 수정(diff)
def querydslDir = layout.buildDirectory.dir("generated/querydsl").get().asFile - tasks.withType(JavaCompile).configureEach { - options.generatedSourceOutputDirectory.set(querydslDir) - } + tasks.named('compileJava') { + options.generatedSourceOutputDirectory.set(querydslDir) + } + // 테스트용 Q-타입은 별도 디렉터리로 분리 가능: + // tasks.named('compileTestJava') { + // options.generatedSourceOutputDirectory.set( + // layout.buildDirectory.dir("generated/querydslTest").get().asFile + // ) + // }검증 요청
프로젝트를 실제로 빌드한 후(./gradlew compileJava compileTestJava),
build/generated/querydsl디렉터리 내에 테스트 전용 Q-타입 파일이 섞여 있는지 직접 확인해 주세요.src/main/java/project/flipnote/group/repository/GroupMemberRepositoryCustom.java (1)
7-9: 반환 타입 명확성을 드러내는 네이밍/주석 제안
findGroupMembers(Long groupId)는 엔티티가 아닌 DTO(GroupMemberInfo)를 반환합니다. 혼동 방지를 위해 JavaDoc 추가 또는 메서드명을findGroupMemberInfos(Long groupId)처럼 더 구체화하는 것을 고려해볼 수 있습니다. 기능상 문제는 없습니다.예시:
/** * 주어진 그룹의 멤버 정보를 DTO로 조회한다. * @param groupId 그룹 ID * @return GroupMemberInfo 목록 */ List<GroupMemberInfo> findGroupMembers(Long groupId);src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (1)
28-29: 엔티티 조회 메서드 사용 여부 검증 및 네이밍 일관성 제안검증 결과, 새로 추가된
GroupMemberRepository.findAllByGroup_Id(Long)는 코드 내에서 호출되지 않습니다. 따라서 당장 사용처가 없다면 API 증가를 막기 위해 해당 메서드 추가를 보류하거나 삭제를 고려해 주세요.또한, 기존 메서드들의
Id대소문자 표기 규칙이 혼재되어 있어 가독성이 떨어집니다. 아래 항목을 참고해 이후 리팩터링을 권장드립니다.
- 호출부 확인 결과
findAllByGroup_Id호출부 없음
(rg -n '\bfindAllByGroup_Id\s*\(' -C2 src/main/java결과)- 네이밍 일관성
existsByGroup_idAndUser_id,countByGroup_idAndUser_idNot- 신규 메서드:
findAllByGroup_Id
→ 모두Id대문자 표기(예:Group_Id,User_Id) 또는 소문자 표기(예:group_id,user_id) 중 하나로 통일src/test/java/project/flipnote/group/service/GroupServiceTest.java (2)
73-75: 불필요한 Mock 제거 제안 (GroupMemberRepositoryImpl는 사용되지 않음)
@Mock GroupMemberRepositoryImpl는 현재 테스트 어디에서도 사용되지 않습니다. 혼동을 줄이기 위해 제거하는 것이 좋습니다.다음과 같이 정리 가능합니다:
- @Mock - GroupMemberRepositoryImpl groupMemberRepositoryImpl;
317-351: 그룹 멤버 조회 테스트 전반은 적절합니다. 다만 ID 의미와 테스트 안정성 확인 부탁드립니다.
- 시나리오/검증은 명확합니다. 저장소 상호작용 검증도 OK입니다.
GroupMemberInfo.from(userProfile.getId(), ...)로 내려가는id가 "사용자 ID"임을 전제하고 있습니다. API에서id가 "그룹멤버 엔티티 ID"가 아닌 "사용자 ID"임이 의도인지 확인 부탁드립니다. 이후 변경이 어렵기 때문에 합의가 필요합니다.- 테스트 안정성 관점에서
UserFixture.createActiveUser()가 ID를 세팅하지 않는 경우,authPrinciple.userId()가 null이 되어 저장소 스텁과 불일치할 수 있습니다. 픽스처가 ID를 보장하지 않는다면, 아래와 같이 명시 세팅을 권장합니다.// before() 내 보완 예시 ReflectionTestUtils.setField(userProfile, "id", 1L); authPrinciple = new AuthPrinciple(1L, 1L, userProfile.getEmail(), AccountRole.USER, 1L);src/main/java/project/flipnote/group/controller/GroupController.java (1)
17-19: 중복 import 제거
FindGroupMemberResponse가 두 번 import 되었습니다. 한 줄을 제거해 주세요.import project.flipnote.group.model.FindGroupMemberResponse; -import project.flipnote.group.model.FindGroupMemberResponse;src/main/java/project/flipnote/group/model/GroupMemberInfo.java (1)
5-14: 필드 명확성 개선: id → userId, profile → profileImageUrl (권장)현재
id가 사용자 ID를 의미합니다. 외부 API 스키마에서 혼동을 줄이려면 보다 구체적인 필드명이 좋습니다. 새 기능이므로 지금 변경하는 편이 장기 유지보수에 유리합니다.-public record GroupMemberInfo( - Long id, - GroupMemberRole role, - String name, - String profile -) { - public static GroupMemberInfo from(Long id, GroupMemberRole role, String name, String profile) { - return new GroupMemberInfo(id, role, name, profile); +public record GroupMemberInfo( + Long userId, + GroupMemberRole role, + String name, + String profileImageUrl +) { + public static GroupMemberInfo from(Long userId, GroupMemberRole role, String name, String profileImageUrl) { + return new GroupMemberInfo(userId, role, name, profileImageUrl); } }추가로 아래 사용처도 함께 업데이트가 필요합니다(예시):
- GroupMemberRepositoryImpl: 생성자 프로젝션 인자 순서/이름
- GroupServiceTest:
res.groupMembers().get(0).userId()로 검증 변경src/main/java/project/flipnote/group/model/FindGroupMemberResponse.java (1)
5-11: 응답 리스트 방어적 복사 제안외부에서 전달된
List가 가변인 경우를 방지하려면 정적 팩토리에서 방어적 복사를 권장합니다.public record FindGroupMemberResponse( List<GroupMemberInfo> groupMembers ) { public static FindGroupMemberResponse from(List<GroupMemberInfo> groupMembers) { - return new FindGroupMemberResponse(groupMembers); + return new FindGroupMemberResponse(List.copyOf(groupMembers)); } }src/main/java/project/flipnote/group/repository/GroupMemberRepositoryImpl.java (2)
22-34: 시그니처 구현 표식과 정렬 추가 제안
- 인터페이스 구현 이후
@Override를 추가하면 오타/시그니처 변경 시 컴파일 타임에 잡힙니다.- 대규모 결과에 대한 일관 응답을 위해 정렬 기준(예: 역할, 이름)을 부여하는 것을 고려해 보세요.
- public List<GroupMemberInfo> findGroupMembers(Long groupId) { + @Override + public List<GroupMemberInfo> findGroupMembers(Long groupId) { return queryFactory.select(Projections.constructor( GroupMemberInfo.class, userProfile.id, groupMember.role, userProfile.name, userProfile.profileImageUrl )) .from(groupMember) .join(groupMember.user, userProfile) .where(groupMember.group.id.eq(groupId)) + // 필요 시 정렬 예시: + // .orderBy(groupMember.role.asc(), userProfile.name.asc()) .fetch(); }
19-21: Q타입 상수화로 의도 명확화 (선택사항)Q타입은 상태가 없으므로
static final로 선언하면 의도가 선명해집니다.-QUserProfile userProfile = QUserProfile.userProfile; -QGroupMember groupMember = QGroupMember.groupMember; +private static final QUserProfile userProfile = QUserProfile.userProfile; +private static final QGroupMember groupMember = QGroupMember.groupMember;src/main/java/project/flipnote/group/service/GroupService.java (4)
56-63: 중복 로직 제거: 존재 검증은 getGroupMember 재사용 권장
validateGroupInUser와getGroupMember가 동일한 쿼리와 동일한 예외 흐름을 갖습니다. DRY 관점에서validateGroupInUser가getGroupMember를 호출하도록 단순화하면 유지보수성이 좋아집니다. 또한 메서드 명은 일반적으로validateUserInGroup가 더 자연스럽습니다. (리네임은 선택사항)아래와 같이 단순화할 수 있습니다:
public void validateGroupInUser(UserProfile user, Long groupId) { - groupMemberRepository.findByGroup_IdAndUser_Id(groupId, user.getId()).orElseThrow( - () -> new BizException(GroupJoinErrorCode.USER_NOT_IN_GROUP) - ); + // 존재 검증 로직을 단일 지점으로 집약 + getGroupMember(user, groupId); }
245-259: 멤버 리스트 접근 권한 재검토 및 테스트 보강 제안현재는 "그룹에 속해 있으면 모두 조회 가능" 정책입니다. 정책적으로 멤버 목록 열람 권한이 별도로 존재한다면(예: MEMBER_READ/USER_LIST_VIEW 등),
hasPermission(groupId, user.getId(), <권한상수>)체크를 추가해 주세요. 또한 노출 필드가 PII에 해당하지 않는지(DTO에 이메일/전화번호 등 포함 여부) 점검 바랍니다.테스트 측면에서는 다음 케이스를 권장합니다:
- 비멤버가 요청 시 USER_NOT_IN_GROUP 에러 반환
- 삭제되었거나 존재하지 않는 그룹 요청 시 GROUP_NOT_FOUND 반환
- 응답 정렬 기준이 있다면 정렬 일관성 검증
원하시면 권한 체크 및 위 음수 케이스(403/404 등) 단위/통합 테스트 템플릿을 추가로 드리겠습니다.
68-72: GroupService 내부 에러 코드 통일 및 테스트 수정 안내아래 파일에서
GroupJoinErrorCode.USER_NOT_IN_GROUP대신GroupErrorCode.USER_NOT_IN_GROUP을 사용하도록 리팩터링하고, 이에 맞춰 테스트도 함께 업데이트해주세요.• src/main/java/project/flipnote/group/service/GroupService.java
- validateGroupInUser (라인 61)
- getGroupMember (라인 70)
return groupMemberRepository.findByGroup_IdAndUser_Id(groupId, user.getId()).orElseThrow( - () -> new BizException(GroupJoinErrorCode.USER_NOT_IN_GROUP) + () -> new BizException(GroupErrorCode.USER_NOT_IN_GROUP) );• src/test/java/project/flipnote/group/service/GroupServiceTest.java (라인 196~198)
- import 구문 변경
- assertion 수정
-import project.flipnote.groupjoin.error.GroupJoinErrorCode; +import project.flipnote.group.error.GroupErrorCode; ... -assertEquals(GroupJoinErrorCode.USER_NOT_IN_GROUP, exception.getErrorCode()); +assertEquals(GroupErrorCode.USER_NOT_IN_GROUP, exception.getErrorCode());위 변경으로 GroupService 내부에서 사용하는 에러 코드를
GroupErrorCode로 일관되게 유지하고, 테스트 오류를 방지할 수 있습니다.
188-195: 리팩토링 제안: private 메서드명 변경 및 정렬 보장 검토명확성을 위해 Service 내 오버로드된
findGroupMembers(AuthPrinciple, Long)메서드와 충돌하는 private 메서드를loadGroupMemberInfos(또는findGroupMemberInfos)로 변경하시길 권장합니다. 예:- /* - 그룹 내 모든 멤버리스트 조회 - */ - private List<GroupMemberInfo> findGroupMembers(Long groupId) { - //각 그룹멤버의 id를 가지고 유저를 찾고 유저명, 권한, 등등 가져오기 - return groupMemberRepository.findGroupMembers(groupId); - } + /* + 그룹 내 모든 멤버 정보 로드 + */ + private List<GroupMemberInfo> loadGroupMemberInfos(Long groupId) { + return groupMemberRepository.findGroupMembers(groupId); + }- List<GroupMemberInfo> groupMembers = findGroupMembers(groupId); + List<GroupMemberInfo> groupMembers = loadGroupMemberInfos(groupId);또한,
GroupMemberRepositoryImpl#findGroupMembers는 QueryDSL의 DTO 프로젝션 및join(groupMember.user, userProfile)으로 N+1 이슈를 방지하고 있으나, 현재 반환 순서가 미정의 상태입니다. 일관된 순서 보장이 필요하다면 아래와 같이.orderBy(...)를 추가하세요:return queryFactory .select(Projections.constructor( GroupMemberInfo.class, userProfile.id, groupMember.role, userProfile.name, userProfile.profileImageUrl )) .from(groupMember) .join(groupMember.user, userProfile) .where(groupMember.group.id.eq(groupId)) .orderBy( // 예: 역할 순서(desc) → 가입일(오름차순) groupMember.role.desc(), userProfile.createdAt.asc() ) .fetch();
- 그룹 최대 인원(100명) 기준으로 페이지네이션은 필수는 아니나, 향후 확장 시 고려하시기 바랍니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
.gitignore(1 hunks)build.gradle(1 hunks)src/main/java/project/flipnote/group/controller/GroupController.java(2 hunks)src/main/java/project/flipnote/group/model/FindGroupMemberResponse.java(1 hunks)src/main/java/project/flipnote/group/model/GroupMemberInfo.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupMemberRepository.java(2 hunks)src/main/java/project/flipnote/group/repository/GroupMemberRepositoryCustom.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupMemberRepositoryImpl.java(1 hunks)src/main/java/project/flipnote/group/service/GroupService.java(5 hunks)src/main/java/project/flipnote/infra/config/QuerydslConfig.java(1 hunks)src/test/java/project/flipnote/group/service/GroupServiceTest.java(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/main/java/project/flipnote/infra/config/QuerydslConfig.java (1)
src/main/java/project/flipnote/group/repository/GroupMemberRepositoryImpl.java (1)
RequiredArgsConstructor(14-36)
src/main/java/project/flipnote/group/controller/GroupController.java (3)
src/main/java/project/flipnote/group/controller/GroupInvitationQueryController.java (1)
RequiredArgsConstructor(19-50)src/main/java/project/flipnote/groupjoin/controller/GroupJoinController.java (1)
RestController(14-76)src/main/java/project/flipnote/group/controller/GroupInvitationController.java (1)
RequiredArgsConstructor(23-63)
src/main/java/project/flipnote/group/model/FindGroupMemberResponse.java (2)
src/main/java/project/flipnote/groupjoin/model/FindGroupJoinListMeResponse.java (2)
FindGroupJoinListMeResponse(7-13)from(10-12)src/main/java/project/flipnote/groupjoin/model/GroupJoinListResponse.java (1)
GroupJoinListResponse(7-13)
src/main/java/project/flipnote/group/model/GroupMemberInfo.java (2)
src/main/java/project/flipnote/group/entity/GroupMember.java (2)
Getter(23-53)Builder(47-52)src/main/java/project/flipnote/group/entity/GroupMemberRole.java (1)
GroupMemberRole(3-5)
src/main/java/project/flipnote/group/service/GroupService.java (2)
src/main/java/project/flipnote/groupjoin/service/GroupJoinService.java (1)
Slf4j(41-286)src/main/java/project/flipnote/group/service/GroupMemberPolicyService.java (1)
RequiredArgsConstructor(22-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
build.gradle (1)
67-73: sourceSets 연결 LGTM생성 디렉터리를
main소스셋에 연결한 방식이 표준적이고 IDE 인식에도 유리합니다.src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (1)
15-15: 커스텀 레포지토리 구현 클래스 정상 인식 확인 완료GroupMemberRepositoryImpl 클래스가
src/main/java/project/flipnote/group/repository패키지에 올바르게 위치해 있으며,findGroupMembers커스텀 메서드가 서비스와 컨트롤러 레이어에서 정상적으로 호출되는 것을 확인했습니다. Spring Data가 커스텀 구현을 인식하는 데 문제가 없으므로 변경 사항을 승인합니다.src/test/java/project/flipnote/group/service/GroupServiceTest.java (1)
297-297: Reflection으로 그룹 ID 주입 OK도메인 생성자를 해치지 않으면서 ID를 부여하는 합리적인 선택입니다.
src/main/java/project/flipnote/group/controller/GroupController.java (1)
60-68: GET /v1/groups/{groupId}/members 엔드포인트 추가 LGTM서비스 위임과 응답 포맷이 명확하고, 기존 컨트롤러 스타일과 일관됩니다.
src/main/java/project/flipnote/group/service/GroupService.java (2)
20-24: DTO import 추가는 적절합니다.새 API(response/vo) 사용을 위한 필요한 의존성만 추가되어 있어 깔끔합니다.
224-224: 메서드 명 변경 반영 OK
deleteGroup에서 기존 조회/검증 호출을getGroupMember로 교체한 부분은 의도가 명확해졌고, 반환값을 즉시 활용 가능해져 좋습니다.
src/main/java/project/flipnote/group/repository/GroupMemberRepositoryImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/main/java/project/flipnote/group/repository/GroupRepository.java (1)
25-27: deletedAt 기반 조회가 빈번하면 인덱스 고려
existsByIdAndDeletedAtIsNull가 트래픽 상위 호출이라면(id, deletedAt)복합 인덱스를 고려해 주세요. RDBMS에 따라 커버링/부분 인덱스로 존재 체크 비용을 더 낮출 수 있습니다.src/test/java/project/flipnote/group/service/GroupServiceTest.java (3)
77-79: 불필요한 구현체(Mock) 제거
@Mock GroupMemberRepositoryImpl를 선언했지만 사용되지 않습니다. 서비스는GroupMemberRepository(인터페이스)에 의존하므로 구현체 모킹은 중복·혼란을 유발합니다. 아래처럼 제거해 주세요.- import project.flipnote.group.repository.GroupMemberRepositoryImpl; ... - @Mock - GroupMemberRepositoryImpl groupMemberRepositoryImpl;Also applies to: 43-44
326-360: 그룹 멤버 조회 테스트 케이스 전반적으로 적절(LGTM) + 검증 강화 제안기능 흐름(그룹 존재 → 유저 활성 → 그룹 멤버십 검증 → 멤버 목록 조회)을 충실히 모킹·검증했습니다. 권한 체크 호출에 대한 검증을 추가하면 테스트 신뢰도가 더 좋아집니다.
then(groupRepository).should().existsByIdAndDeletedAtIsNull(group.getId()); then(groupMemberRepository).should().findGroupMembers(group.getId()); +then(groupMemberRepository).should().existsByGroup_IdAndUser_Id(group.getId(), userProfile.getId());
385-385: 모킹 시 any() 대신 구체 값 사용
existsByIdAndDeletedAtIsNull(any())는 과도하게 관대한 매칭입니다. 명시적 groupId를 사용하면 회귀를 더 잘 포착합니다.- given(groupRepository.existsByIdAndDeletedAtIsNull(any())).willReturn(true); + given(groupRepository.existsByIdAndDeletedAtIsNull(group.getId())).willReturn(true);src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (1)
20-29: 동일 의미의 파생 쿼리 중복 정의 제거 및 파라미터 명료화
existsByGroup_idAndUser_id와existsByGroup_IdAndUser_Id는 동일 의미의 파생 쿼리로 유지 시 혼동을 줍니다. 하나만 남기는 편이 좋습니다. 또한existsByGroup_IdAndUser_Id의 두 번째 파라미터명이id라 모호합니다.userId로 명확화해 주세요.- boolean existsByGroup_idAndUser_id(Long groupId, Long userId); ... - boolean existsByGroup_IdAndUser_Id(Long groupId, Long id); + boolean existsByGroup_IdAndUser_Id(Long groupId, Long userId);추가로 네이밍 일관성 측면에서
countByGroup_idAndUser_idNot도countByGroup_IdAndUser_IdNot형태로의 정렬을 권장합니다(서비스/테스트 호출부 동반 수정 필요).src/main/java/project/flipnote/group/service/GroupService.java (3)
235-242: 동일 시그니처명 혼동 가능성: private 메서드 이름 개선 권장public
findGroupMembers(...)와 privatefindGroupMembers(...)가 공존해 가독성이 떨어질 수 있습니다. private 메서드를fetchGroupMembers(또는loadGroupMembers) 등으로 바꾸는 것을 권장합니다.-private List<GroupMemberInfo> findGroupMembers(Long groupId) { +private List<GroupMemberInfo> fetchGroupMembers(Long groupId) { //각 그룹멤버의 id를 가지고 유저를 찾고 유저명, 권한, 등등 가져오기 return groupMemberRepository.findGroupMembers(groupId); }
303-307: 위 이름 변경에 따른 호출부 정렬상기 메서드명을 변경했다면 호출부도 함께 수정되어야 합니다.
- List<GroupMemberInfo> groupMembers = findGroupMembers(groupId); + List<GroupMemberInfo> groupMembers = fetchGroupMembers(groupId);
292-307: 멤버 리스트 응답: 페이징/정렬 확장 포인트 고려멤버 수가 많은 그룹을 고려하면 서버/네트워크 부하를 줄이기 위해 페이지네이션(
Pageable)과 정렬(역할/이름 등) 파라미터를 노출할 여지가 있습니다. API/리포지토리 시그니처를 확장 가능하게 설계해 두면 향후 변경 비용을 줄일 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/main/java/project/flipnote/group/repository/GroupMemberRepository.java(2 hunks)src/main/java/project/flipnote/group/repository/GroupRepository.java(1 hunks)src/main/java/project/flipnote/group/service/GroupService.java(6 hunks)src/test/java/project/flipnote/group/service/GroupServiceTest.java(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/project/flipnote/group/repository/GroupRepository.java (2)
src/main/java/project/flipnote/groupjoin/repository/GroupJoinRepository.java (2)
existsByGroup_idAndUser_id(18-18)Repository(12-19)src/main/java/project/flipnote/group/repository/GroupInvitationRepository.java (3)
existsByGroup_IdAndInviteeUserIdAndStatus(31-31)GroupInvitationRepository(17-43)existsByGroup_IdAndInviteeEmailAndStatus(33-33)
src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (1)
src/main/java/project/flipnote/groupjoin/repository/GroupJoinRepository.java (1)
Repository(12-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
src/main/java/project/flipnote/group/repository/GroupRepository.java (1)
25-27: 존재 여부 파생 쿼리 추가 적절(LGTM)소프트 삭제(deletedAt) 조건을 포함한 존재 검증용 파생 쿼리
existsByIdAndDeletedAtIsNull(Long groupId)추가가 서비스 계층의 검증 로직 단순화와 성능(불필요한 엔티티 로딩 제거)에 도움이 됩니다.src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (1)
15-15: 커스텀 리포지토리 확장(LGTM)
GroupMemberRepositoryCustom확장으로 QueryDSL 기반 조회를 노출한 방향 좋습니다. 인터페이스 경계를 통해 서비스에서는 구현체 세부사항을 몰라도 됩니다.src/main/java/project/flipnote/group/service/GroupService.java (3)
60-67: 멤버십 검증 메서드 분리 적절(LGTM)
validateGroupInUser로 멤버십 검증을 명확히 분리하여 재사용성과 가독성이 개선됐습니다. 에러코드도GroupJoinErrorCode.USER_NOT_IN_GROUP으로 일관적입니다.
81-85: 그룹 존재 검증 메서드 추가 적절(LGTM)존재만 확인할 때
existsBy...를 사용해 불필요한 엔티티 로딩을 피한 점 좋습니다. 호출부에서 후속 로딩이 필요 없는 경우에 적합합니다.
90-94: 그룹 엔티티 조회 메서드 분리 적절(LGTM)상세가 필요한 경우
getGroup을 통해 not-found를 일관되게 처리합니다. 서비스 메서드별로validateGroup/getGroup의 역할 구분이 명확합니다.
📝 변경 내용
✅ 체크리스트
💬 기타 참고 사항
Summary by CodeRabbit
신기능
Chores
테스트