Feat: [FN-119] 그룹 전체 조회 API 추가 #37
Conversation
Walkthrough패키지 재배치와 JPA 연관 로딩 전략 변경을 포함해, 그룹 목록 조회 기능을 커서 기반 페이징으로 추가했다. 컨트롤러에 GET /v1/groups 엔드포인트를 도입하고, 서비스/리포지토리 계층에 커서 조회 및 카테고리 필터를 구현했고 사용자 검증 흐름을 분리·정리했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as 클라이언트
participant GC as GroupController
participant GS as GroupService
participant UR as UserProfileRepository
participant GR as GroupRepositoryImpl
C->>GC: GET /v1/groups?lastId&category&size
GC->>GS: findGroup(authPrinciple, GroupListRequest)
GS->>UR: existsByIdAndStatus(userId, ACTIVE)
UR-->>GS: boolean exists
alt 사용자 없음
GS-->>GC: throw BizException(INVALID_USER)
GC-->>C: 4xx 응답
else 사용자 존재
GS->>GR: findAllByCursor(lastId, category, pageSize)
GR-->>GS: List<GroupInfo> (size up to pageSize+1)
GS->>GS: compute hasNext, nextCursor, trim list
GS-->>GC: CursorPagingResponse<GroupInfo>
GC-->>C: 200 OK + body
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
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: 3
🧹 Nitpick comments (16)
src/main/java/project/flipnote/group/exception/GroupErrorCode.java (1)
20-20: 사소한 포맷팅: 불필요한 공백 제거문자열 뒤의 공백 후
);는 기존 스타일과 불일치합니다.- INVALID_CATEGORY(HttpStatus.BAD_REQUEST, "GROUP_009", "지원하지 않는 카테고리입니다." ); + INVALID_CATEGORY(HttpStatus.BAD_REQUEST, "GROUP_009", "지원하지 않는 카테고리입니다.");src/main/java/project/flipnote/group/repository/GroupMemberRepositoryImpl.java (2)
18-19: Q타입 필드 정적 상수화로 의도 명확화Q타입은 싱글턴이므로 필드를
private static final로 정의하면 의도가 분명해지고 불필요한 인스턴스 필드로 오해를 줄입니다.- QUserProfile userProfile = QUserProfile.userProfile; - QGroupMember groupMember = QGroupMember.groupMember; + private static final QUserProfile userProfile = QUserProfile.userProfile; + private static final QGroupMember groupMember = QGroupMember.groupMember;
31-33: 결과 정렬 기준 명시 제안현재 반환 순서가 비결정적입니다. 역할/이름 등으로 정렬을 추가하면 응답 안정성과 테스트 재현성이 향상됩니다.
.join(groupMember.user, userProfile) .where(groupMember.group.id.eq(groupId)) + .orderBy(groupMember.role.asc(), userProfile.name.asc()) .fetch();src/main/java/project/flipnote/group/repository/GroupRepositoryCustom.java (1)
8-10: 커서 기반 조회의 계약(정렬, 방향, 페이지 크기)을 인터페이스 수준에서 명확히 해주세요현재 메서드명/시그니처만으로는 정렬 기준(id desc), 커서 비교 연산(id < lastId), 페이지 크기(고정 10) 등의 계약이 드러나지 않습니다. 서비스/컨트롤러가 이를 암묵적으로 가정하면 변경 시 결합도가 올라갑니다.
- Javadoc로 계약을 명시하거나
- size를 파라미터로 노출하거나
- Slice/CursorPage 같은 반환 타입으로 hasNext/next까지 책임지도록 하는 방안을 고려해 주세요.
src/main/java/project/flipnote/group/controller/GroupController.java (2)
14-22: import 정리 및 중복 제거 권장이번 변경으로
@RequestParamimport가 추가되었는데, 이 파일에는PathVariable,FindGroupMemberResponse중복 import가 보입니다. 컴파일에는 문제 없더라도 정리해 두는 편이 좋습니다.
- 불필요/중복 import 제거
- 사용되지 않는 import 정리
86-96: 그룹 목록 조회 파라미터 검증 및 타입 일치화 제안엔드포인트는 깔끔합니다만 몇 가지 보완점을 제안드립니다.
- category를 String이 아닌 Enum으로 직접 바인딩하면 변환/검증 비용을 줄일 수 있습니다. 잘못된 값은 Spring이 곧바로 400을 반환합니다(커스텀 메시지 필요 시 Converter/ControllerAdvice로 보완).
- lastId에 대한 범위 검증(> 0) 추가 권장.
- 페이지 크기(size)를 쿼리 파라미터로 노출하거나, 최소/최대 한도를 두어 DoS성 큰 요청을 방지.
아래는 category를 Enum으로 받는 최소 수정 예시입니다.
- @RequestParam(name = "category", required = false) String category + @RequestParam(name = "category", required = false) Category category추가로 필요한 import(파일 상단):
import project.flipnote.group.entity.Category;또한 PR 체크리스트에 문서 미갱신으로 표시되어 있습니다. OpenAPI/README에 쿼리 파라미터(lastId, category), 응답 스키마(FindGroupResponse)를 반영해 주세요. 원하시면 초안 제공 가능합니다.
src/main/java/project/flipnote/group/model/FindGroupResponse.java (1)
5-13: 불필요한 박싱(Boolean) 제거 및 명확한 필드명 제안(next → nextCursor)
- hasNext는 primitive boolean으로 충분해 보입니다. 박싱(Boolean)은 null 상태를 허용해 불변식이 약해집니다.
- next는 의미상 커서이므로 nextCursor가 더 자명합니다.
제안 diff:
-public record FindGroupResponse( - List<GroupInfo> groups, - Long next, - Boolean hasNext -) { - public static FindGroupResponse from(List<GroupInfo> groups, Long next, Boolean hasNext) { - return new FindGroupResponse(groups, next, hasNext); - } +public record FindGroupResponse( + List<GroupInfo> groups, + Long nextCursor, + boolean hasNext +) { + public static FindGroupResponse from(List<GroupInfo> groups, Long nextCursor, boolean hasNext) { + return new FindGroupResponse(groups, nextCursor, hasNext); + } }이 변경은 컨트롤러/서비스/테스트의 필드명 참조도 함께 수정이 필요합니다.
src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java (2)
22-22: Q 타입 별칭 이름 개선(g → group) 또는 static final로 승격
group은 JPQL 키워드(group by)와 겹쳐 가독성이 떨어질 수 있습니다. 정적 상수로 승격하고 짧은 별칭을 쓰면 가독성이 좋아집니다.제안 diff:
- QGroup group = QGroup.group; + private static final QGroup g = QGroup.group;아래 참조들(
group.*)도g.*로 함께 변경이 필요합니다.
34-36: 조회 패턴을 고려한 인덱스 제안: (category, id DESC) 또는 (deletedAt, category, id)카테고리 필터 + id 역정렬/커서 비교 패턴이 빈번하다면 해당 조합에 맞는 복합 인덱스를 추가하면 성능이 크게 개선될 수 있습니다.
deletedAt IS NULL조건이 상시라면 부분 인덱스(지원 DB 한정)나 선두 컬럼 포함도 검토해 주세요.Also applies to: 48-49
src/main/java/project/flipnote/group/service/GroupService.java (7)
46-46: 페이지 크기 상수는 클래스 상수로 고정하고 가시성 제한상수는 인스턴스마다 가질 필요가 없어
private static final이 적합합니다. 또한 이후 설정값으로 빼는 여지도 남길 수 있습니다.- final int SIZE = 10; + private static final int SIZE = 10;
64-71: validateUser와 getUser의 중복 조회 방지
findGroup처럼 엔티티가 필요 없을 때는validateUser, 그 외에는getUser를 쓰는 의도는 이해됩니다. 다만 동일 요청 흐름에서 두 메서드가 연속 호출되어 DB를 2번 타는 사용처가 없는지 확인 필요합니다. 필요 시, 하나의 메서드로 통합하거나 내부에서 조건적으로 존재 여부/엔티티를 선택 조회하는 방식으로 묶는 것도 고려해볼 만합니다.
240-246: 불리언 의미가 반대처럼 읽히는 메서드명 정리현재 구현은 “오너 외 인원이 존재하면 false, 없으면 true”를 반환하지만,
checkUserNotExistInGroup라는 이름은 오해를 유발할 수 있습니다. 의미가 드러나는 이름과 단순 비교로 정리하는 것을 제안합니다.- private boolean checkUserNotExistInGroup(UserProfile user, Group group) { - long count = groupMemberRepository.countByGroup_idAndUser_idNot(group.getId(), user.getId()); - if (count > 0) { - return false; - } - return true; - } + private boolean areOtherMembersAbsent(UserProfile owner, Group group) { + long others = groupMemberRepository.countByGroup_idAndUser_idNot(group.getId(), owner.getId()); + return others == 0; + }
291-299: 삭제 흐름 검토: 연관/소프트삭제 정책 확인 + 호출부 이름 변경
- 현재
groupMember를 삭제 후group을 삭제합니다. 엔티티에@SQLDelete(소프트 삭제)나 적절한 cascade 설정이 되어 있지 않다면 물리 삭제가 일어날 수 있어, 서비스 전반에서 사용 중인deletedAt IS NULL필터와 충돌할 여지가 있습니다. Group/GroupMember/GroupRolePermission 등 연관 항목의 삭제/보존 정책을 재확인 부탁드립니다.- 앞선 네이밍 개선에 따라 호출부도 함께 정리되면 가독성이 좋아집니다.
- if (!checkUserNotExistInGroup(user, group)) { + if (!areOtherMembersAbsent(user, group)) { throw new BizException(GroupErrorCode.OTHER_USER_EXIST_IN_GROUP); }
340-350: 카테고리 파싱의 Locale 안전성 및 빠른 반환
toUpperCase()는 Locale 민감합니다.Locale.ROOT사용을 권장합니다. 또한 빈 입력은 초기에 빠르게 반환하면 가독성이 좋아집니다.- private Category convertCategory(String rawCategory) { - Category category = null; - if (rawCategory != null && !rawCategory.isBlank()) { - try { - category = Category.valueOf(rawCategory.trim().toUpperCase()); - } catch (IllegalArgumentException e) { - throw new BizException(GroupErrorCode.INVALID_CATEGORY); - } - } - return category; - } + private Category convertCategory(String rawCategory) { + if (rawCategory == null || rawCategory.isBlank()) { + return null; + } + try { + return Category.valueOf(rawCategory.trim().toUpperCase(java.util.Locale.ROOT)); + } catch (IllegalArgumentException e) { + throw new BizException(GroupErrorCode.INVALID_CATEGORY); + } + }
324-350: 신규 조회 API 테스트 커버리지 제안
findGroup/convertCategory관련 경계 조건 테스트를 추가하면 회귀 방지에 도움이 됩니다.권장 케이스:
- lastId가 null/유효/경계(마지막 페이지)일 때 각각
hasNext,nextCursor가 올바른지.- 카테고리가 null/blank/유효/무효일 때 필터/에러 응답이 올바른지.
- SIZE보다 1건 많은 결과에서 중복/누락 없이 다음 페이지로 이어지는지.
원하시면 Mockito 기반 단위 테스트 스켈레톤을 드리겠습니다.
228-230: 주석 오타 수정 제안“권환” → “권한”
Also applies to: 286-288
📜 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 (12)
src/main/java/project/flipnote/common/config/QuerydslConfig.java(1 hunks)src/main/java/project/flipnote/group/controller/GroupController.java(2 hunks)src/main/java/project/flipnote/group/entity/GroupRolePermission.java(1 hunks)src/main/java/project/flipnote/group/exception/GroupErrorCode.java(1 hunks)src/main/java/project/flipnote/group/model/FindGroupResponse.java(1 hunks)src/main/java/project/flipnote/group/model/GroupInfo.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupMemberRepositoryImpl.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupRepository.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupRepositoryCustom.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java(1 hunks)src/main/java/project/flipnote/group/service/GroupService.java(12 hunks)src/main/java/project/flipnote/user/repository/UserProfileRepository.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
src/main/java/project/flipnote/group/repository/GroupRepository.java (2)
src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (1)
Repository(14-29)src/main/java/project/flipnote/group/repository/GroupMemberRepositoryCustom.java (2)
GroupMemberRepositoryCustom(7-9)findGroupMembers(8-8)
src/main/java/project/flipnote/group/model/FindGroupResponse.java (3)
src/main/java/project/flipnote/group/model/FindGroupMemberResponse.java (1)
FindGroupMemberResponse(5-11)src/main/java/project/flipnote/groupjoin/model/FindGroupJoinListMeResponse.java (1)
FindGroupJoinListMeResponse(7-13)src/main/java/project/flipnote/groupjoin/model/GroupJoinListResponse.java (1)
GroupJoinListResponse(7-13)
src/main/java/project/flipnote/user/repository/UserProfileRepository.java (3)
src/main/java/project/flipnote/groupjoin/repository/GroupJoinRepository.java (2)
Repository(12-19)existsByGroup_idAndUser_id(18-18)src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (2)
existsByGroup_idAndUser_id(20-20)existsByGroup_IdAndUser_Id(28-28)src/main/java/project/flipnote/auth/repository/UserAuthRepository.java (1)
existsByEmailAndStatus(19-19)
src/main/java/project/flipnote/group/repository/GroupRepositoryCustom.java (3)
src/main/java/project/flipnote/group/repository/GroupMemberRepositoryCustom.java (2)
GroupMemberRepositoryCustom(7-9)findGroupMembers(8-8)src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (1)
Repository(14-29)src/main/java/project/flipnote/notification/repository/NotificationRepository.java (1)
NotificationRepository(15-45)
src/main/java/project/flipnote/group/controller/GroupController.java (2)
src/main/java/project/flipnote/groupjoin/controller/GroupJoinController.java (1)
RestController(14-76)src/main/java/project/flipnote/group/controller/GroupInvitationQueryController.java (1)
RequiredArgsConstructor(19-50)
src/main/java/project/flipnote/group/entity/GroupRolePermission.java (3)
src/main/java/project/flipnote/group/entity/GroupPermission.java (1)
Entity(12-29)src/main/java/project/flipnote/group/repository/GroupRolePermissionRepository.java (2)
Repository(13-18)findByGroupAndGroupPermission(17-17)src/main/java/project/flipnote/group/entity/GroupMember.java (1)
Getter(24-52)
src/main/java/project/flipnote/group/repository/GroupMemberRepositoryImpl.java (3)
src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (2)
Repository(14-29)findByGroup_IdAndUser_Id(22-22)src/main/java/project/flipnote/group/repository/GroupMemberRepositoryCustom.java (2)
GroupMemberRepositoryCustom(7-9)findGroupMembers(8-8)src/test/java/project/flipnote/group/service/GroupServiceTest.java (1)
Test(326-360)
src/main/java/project/flipnote/group/exception/GroupErrorCode.java (3)
src/main/java/project/flipnote/group/exception/GroupInvitationErrorCode.java (1)
Getter(9-25)src/main/java/project/flipnote/groupjoin/exception/GroupJoinErrorCode.java (1)
Getter(8-26)src/main/java/project/flipnote/cardset/exception/CardSetErrorCode.java (1)
Getter(9-23)
src/main/java/project/flipnote/common/config/QuerydslConfig.java (2)
src/main/java/project/flipnote/infra/config/QuerydslConfig.java (2)
Configuration(11-20)Bean(16-19)src/main/java/project/flipnote/common/config/AuditingConfig.java (1)
AuditingConfig(6-10)
src/main/java/project/flipnote/group/service/GroupService.java (1)
src/test/java/project/flipnote/group/service/GroupServiceTest.java (4)
Test(362-403)Test(151-182)ExtendWith(52-475)groupService(433-433)
🔇 Additional comments (14)
src/main/java/project/flipnote/group/entity/GroupRolePermission.java (2)
27-29: 지연 로딩(LAZY)로 인한 즉시 위험 없음 확인rg 검색 결과,
getGroupPermission().getName()호출 지점이 코드에 존재하지 않아 현재 LIE(LazyInitializationException)나 N+1 문제가 발생할 가능성이 없습니다.List<GroupRolePermission>를 대량으로 조회하는 부분은GroupService.initializeGroupPermissions에서 새로운 엔티티를 빌더로 생성하는 로직으로, 기존 엔티티의 지연 로딩이 트리거되지 않습니다.따라서 이번 변경으로 인한 즉각적인 지연 로딩 이슈는 없으며, 권고드린 fetch join/DTO 전환, equals/hashCode 개선은 향후 권한정보를 노출하거나 직접 직렬화할 필요가 생길 때 적용해 주시면 됩니다.
23-25: 트랜잭션 경계 내group접근 검증 및 N+1 방지 필요LAZY 전환은 적절하지만, 아래 위치들에서
group필드를 접근할 때 트랜잭션 범위를 벗어나면LazyInitializationException이 발생할 수 있고, 반복 조회 시 N+1 이슈가 발생할 수 있으니 반드시 점검하세요.spring.jpa.open-in-view설정이 명시되어 있지 않아 Spring Boot 기본값(true)이 적용될 수 있으나, 운영 환경에서는 다를 수 있으므로 직접 검증을 권장드립니다.점검 대상 위치:
- 서비스 레이어:
GroupJoinService.java
- 168행:
.map(GroupRolePermission::getRole)(권한 조회 시 연관 엔티티 지연 로딩 확인)- 172행:
.map(groupMember -> groupMember.getGroup().getId())(반복 매핑 중group접근)- 서비스 레이어:
GroupInvitationService.java(약 204행):invitation.getGroup().getId()- DTO 매핑:
IncomingGroupInvitationResponse.java(약 21행):invitation.getGroup().getId()권장 대응:
- 위 위치들이 모두 트랜잭션 범위 내에서 실행되는지 확인
- 일괄 조회(scrollable API 등) 또는 목록 반환 시 JPQL fetch join(
JOIN FETCH) 혹은 전용 프로젝션(@query + DTO) 적용으로 N+1 방지spring.jpa.open-in-view를 비활성화할 계획이라면, Controller/Mapper 단계에서 엔티티 접근이 없는지 재검증선택 사항:
@ManyToOne(fetch = FetchType.LAZY, optional = false)로 JPA 설정과 DB 제약조건 일치- Hibernate를 사용 중이라면
@LazyToOne(LazyToOneOption.PROXY)병행으로 의도 명시src/main/java/project/flipnote/user/repository/UserProfileRepository.java (2)
29-29: 추가된 existsByIdAndStatus 메서드 적합성 확인파생 쿼리 네이밍과 시그니처 모두 일관적이며,
findByIdAndStatus대비 경량 존재 여부 체크 용도로 유용합니다.
20-24: 확인:existsByIdAndStatus와findByIdAndStatus는 용도가 명확히 구분됩니다.두 메서드는 반환 타입과 사용 의도가 달라 중복이 아닙니다.
existsByIdAndStatus(Long, UserStatus)
• 불필요한 엔티티 조회 없이 단순 존재 여부만 확인할 때 사용
• 실제 사용 예:GroupService.validateUser()에서 사용자 활성 여부 체크 (GroupService.java:68)findByIdAndStatus(Long, UserStatus)
• 활성 사용자 엔티티를 조회한 뒤 그 정보를 기반으로 로직을 수행할 때 사용
• 여러 서비스(예: UserService, GroupJoinService, CardSetService)와 수십 건의 테스트에서 엔티티 반환을 전제하고 호출따라서 두 메서드를 하나의 패턴으로 통일하기보다, 필요에 따라 존재 확인용
exists…와 엔티티 조회용find…를 그대로 유지하는 것이 적절합니다. 제안된 리팩토링은 오히려 각 사용처에서 의도를 불명확하게 만들 우려가 있습니다.Likely an incorrect or invalid review comment.
src/main/java/project/flipnote/common/config/QuerydslConfig.java (2)
1-20: QuerydslConfig 패키지 이동 적절공용 설정으로 패키지 이동이 합리적이며, 빈 정의도 변경 없이 안전합니다.
1-1: 중복 QuerydslConfig 클래스 존재 여부 검증 완료
src/main/java/project/flipnote/common/config/QuerydslConfig.java파일만 존재src/main/java/project/flipnote/infra/config/QuerydslConfig.java는 리포지토리 내에 없음중복 빈 정의 이슈가 발생하지 않으므로 해당 리뷰 코멘트는 해결되었습니다.
src/main/java/project/flipnote/group/exception/GroupErrorCode.java (1)
20-20: INVALID_CATEGORY 추가 적절카테고리 유효성 검증에 필요한 에러 코드가 적시에 추가되었습니다.
src/main/java/project/flipnote/group/repository/GroupMemberRepositoryImpl.java (1)
21-21: @OverRide 추가 👍인터페이스 계약에 대한 명시가 되어 IDE/컴파일 타임 검증에 유리합니다.
src/main/java/project/flipnote/group/repository/GroupRepository.java (2)
15-15: 커스텀 리포지토리 확장 적절
GroupRepositoryCustom확장으로 QueryDSL 기반 커서 조회 노출이 자연스럽게 연결됩니다.
15-26: 커스텀 리포지토리 구현 바인딩 확인 완료
src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java에서
public class GroupRepositoryImpl implements GroupRepositoryCustom선언 확인src/main/java/project/flipnote/group/repository/GroupRepositoryCustom.java에서
List<GroupInfo> findAllByCursor(Long lastId, Category category)시그니처 일치 확인위 사항을 통해 Spring Data 커스텀 리포지토리 구현이 올바르게 바인딩되어 있으므로 추가 조치가 필요 없습니다.
src/main/java/project/flipnote/group/model/GroupInfo.java (1)
5-14: LGTM – 가벼운 조회용 프로젝션으로 적절합니다Record + 정적 팩토리 패턴이 일관적이고, QueryDSL constructor projection과도 타입 순서가 자연스럽게 매핑됩니다.
src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java (1)
27-36: 비즈니스 필터 재확인: 공개 범위/상태 조건 필요 여부
deletedAt is null,category필터는 적절합니다. 다만 "그룹 전체 조회"가 비공개/승인 필요 그룹을 포함하는지, 혹은 공개 그룹만 노출해야 하는지 정책 확인이 필요해 보입니다. 엔티티에 공개 여부/상태 플래그가 있다면 where 절에 포함시키는 것을 고려해 주세요.src/main/java/project/flipnote/group/service/GroupService.java (2)
58-62: getUser 구현 적절 👍활성 사용자만 조회하고, 미존재 시 도메인 예외로 매핑하는 흐름이 명확합니다. 컨트롤러/서비스 상위에서
authPrinciplenull이 들어오지 않도록만 보장되면 충분해 보입니다.
116-117: 유저 로딩 일관화(validate → getUser) 전환 👍엔티티가 필요한 흐름들(create/change/detail/delete/findMembers)에서
getUser로 일관화한 점 좋습니다. 호출부 가독성이 좋아지고, 이후 권한/소유자 검증 로직 확장도 수월해집니다.Also applies to: 216-217, 265-266, 280-281, 313-314
src/main/java/project/flipnote/group/exception/GroupErrorCode.java
Outdated
Show resolved
Hide resolved
src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/java/project/flipnote/group/exception/GroupErrorCode.java (1)
19-20: 문구 개선 및 신규 에러코드 추가 반영 완료 — LGTM
- INVALID_MEMBER_COUNT 메시지가 자연스럽고 명확하게 개선되었습니다.
- INVALID_CATEGORY 추가로 서비스 계층의 카테고리 검증 시그널이 분명해졌습니다.
src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java (1)
22-47: SIZE+1 페치로 hasNext 판정 정확도 향상 — 제안 반영 확인
.limit(pageSize + 1)로 한 번의 쿼리로 hasNext를 판단할 수 있도록 구현되어 서비스-리포지토리 간 동기화 이슈와 마지막 페이지 오탐(false positive)이 해소됩니다. 정렬 기준(id DESC)과 커서 비교(id < lastId) 방향도 일관적입니다. Good.
🧹 Nitpick comments (5)
src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java (1)
23-26: pageSize 유효성 검증 추가 권장현재 서비스에서 상수로만 전달되어 실사용상 문제는 없지만, 저장소 레벨에서도 방어 코드를 두면 오용에 강해집니다(음수/0 전달 등).
다음과 같이 간단한 가드를 추가하는 것을 제안합니다.
public List<GroupInfo> findAllByCursor(Long lastId, Category category, int pageSize) { + if (pageSize <= 0) { + throw new IllegalArgumentException("pageSize must be positive"); + } BooleanBuilder where = new BooleanBuilder() .and(group.deletedAt.isNull());src/main/java/project/flipnote/group/service/GroupService.java (4)
331-344: SIZE 상수와 pageSize 혼용 — pageSize로 일관화동일 의미의 값이지만, 가독성과 유지보수를 위해 한 변수(pageSize)로 일관되게 사용하는 편이 안전합니다.
다음과 같이 수정해주세요.
- boolean hasNext = groups.size() > SIZE; + boolean hasNext = groups.size() > pageSize; - if (hasNext) { - groups = groups.subList(0, SIZE); - } + if (hasNext) { + groups = groups.subList(0, pageSize); + }
64-71: 주석 내용과 메서드 역할 불일치(검증 vs 조회)
validateUser는 존재 검증이므로 주석을 정정하면 의도가 더 명확합니다.- /* - 유저 정보 조회 - */ + /* + 유저 존재 검증 + */ public void validateUser(AuthPrinciple authPrinciple) {
240-246: 불리언 메서드 네이밍 명확화 제안
checkUserNotExistInGroup는 “부정형 + check” 조합으로 호출부에서 의미 파악이 한 번 더 필요합니다. 반환 의미에 맞춰 긍정형으로 바꾸면 가독성이 좋아집니다.다음과 같이 이름을 바꾸고 호출부를 함께 변경하는 것을 제안합니다(메서드 스코프가 private라 리스크 낮음).
- private boolean checkUserNotExistInGroup(UserProfile user, Group group) { + private boolean hasNoOtherMembers(UserProfile user, Group group) { long count = groupMemberRepository.countByGroup_idAndUser_idNot(group.getId(), user.getId()); if (count > 0) { return false; } return true; }- if (!checkUserNotExistInGroup(user, group)) { + if (!hasNoOtherMembers(user, group)) { throw new BizException(GroupErrorCode.OTHER_USER_EXIST_IN_GROUP); }Also applies to: 291-296
324-344: 커서 페이징 로직 전체 — 동작 방식 적합, 경계 조건 테스트 권장
- 유효 사용자 검증 → 카테고리 변환(무/공백 시 전체) → pageSize+1 조회 → 트리밍 → nextCursor 산출의 흐름이 정석입니다. LGTM.
- 경계 조건(데이터 0건, 정확히 pageSize건, 마지막 페이지 등)에 대한 단위/통합 테스트를 추가하면 회귀를 막을 수 있습니다.
원하시면 다음 시나리오를 포함한 테스트 템플릿(JUnit + MockMvc/슬라이스 테스트) 작성 도와드리겠습니다.
- lastId=null, 데이터 0건 → hasNext=false, nextCursor=null
- lastId=null, 데이터 pageSize건 → 저장소는 pageSize+1을 반환하지 않으면 hasNext=false
- lastId=null, 데이터 pageSize+1건 → hasNext=true, nextCursor=트리밍된 마지막 id
- invalid category → BizException(INVALID_CATEGORY)
📜 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/exception/GroupErrorCode.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupRepositoryCustom.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java(1 hunks)src/main/java/project/flipnote/group/service/GroupService.java(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/project/flipnote/group/repository/GroupRepositoryCustom.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java (2)
src/main/java/project/flipnote/group/repository/GroupRepository.java (1)
Repository(14-27)src/main/java/project/flipnote/group/repository/GroupMemberRepositoryImpl.java (1)
RequiredArgsConstructor(14-36)
src/main/java/project/flipnote/group/exception/GroupErrorCode.java (1)
src/main/java/project/flipnote/group/entity/Group.java (2)
Getter(30-119)validateMaxMemberUpdatable(99-103)
src/main/java/project/flipnote/group/service/GroupService.java (1)
src/main/java/project/flipnote/group/service/GroupPolicyService.java (1)
Service(18-54)
⏰ 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 (1)
src/main/java/project/flipnote/group/service/GroupService.java (1)
324-344: 일관성 검증 완료:findAllByCursor시그니처 일치검증 결과,
GroupRepositoryCustom인터페이스,GroupRepositoryImpl구현체, 그리고GroupService내 호출부가 모두 동일한 시그니처(Long lastId, Category category, int pageSize)를 사용하고 있으며, 레거시 호출이나 불일치된 호출부는 존재하지 않습니다.
–GroupRepositoryCustom.findAllByCursor(Long, Category, int)
–GroupRepositoryImpl.findAllByCursor(Long, Category, int)
–GroupService.findGroup(...)에서의groupRepository.findAllByCursor(lastId, category, pageSize)호출이상 문제 없으므로 해당 부분은 추가 수정이 필요 없습니다.
src/main/java/project/flipnote/group/controller/GroupController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
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/project/flipnote/group/service/GroupService.java (1)
241-247: Spring Data 메서드 명명 오류 가능성(countByGroup_idAndUser_idNot)다른 곳에서 사용한 existsByGroup_IdAndUser_Id와 달리 여기서는 group_id/user_id 소문자+언더스코어를 사용하고 있어 파싱 실패/컴파일 에러 위험이 큽니다. CamelCase로 통일하세요.
- long count = groupMemberRepository.countByGroup_idAndUser_idNot(group.getId(), user.getId()); + long count = groupMemberRepository.countByGroup_IdAndUser_IdNot(group.getId(), user.getId());추가로 메서드가 리포지토리에 실제 정의되어 있는지 확인이 필요합니다.
♻️ Duplicate comments (1)
src/main/java/project/flipnote/group/controller/GroupController.java (1)
87-96: 그룹 전체 조회 엔드포인트 추가 👍@ModelAttribute + CursorPagingResponse 조합으로 기존 컨벤션과 일치합니다. past review에서 제안된 CursorPaging 사용도 반영됨.
🧹 Nitpick comments (7)
src/main/java/project/flipnote/group/model/GroupListRequest.java (2)
16-19: 커서 정렬 고정(역순 id) 선택 적절 — 다만 상위 sort 파라미터는 무시됨을 명시 필요Cursor 기반 일관성을 위해 id DESC 고정은 👍. 하지만 부모 클래스(CursorPagingRequest)의 sortBy/order 필드는 여전히 공개되어 있어, 클라이언트가 전달해도 무시됩니다. 스웨거 문서에 “sortBy/order는 무시됨”을 명시하거나 해당 필드를 숨기는 방안을 검토하세요.
가능한 개선:
- 컨트롤러 파라미터에 @parameter(hidden = true)로 sortBy/order를 숨김 처리
- 혹은 본 요청 DTO를 CursorPagingRequest에서 분리(커서+size만 노출)
14-14: category 입력값에 대한 스키마 힌트 제공서비스에서 enum 변환/검증을 수행하므로 런타임 안전성은 확보됐습니다. 다만 OpenAPI에 허용 값이 드러나지 않아 DX가 떨어집니다. Category enum 값에 맞춰 allowableValues를 선언해 주세요.
예시(실제 enum 값으로 교체 필요):
+import io.swagger.v3.oas.annotations.media.Schema; @@ - private String category; + @Schema(description = "그룹 카테고리", example = "HOBBY", allowableValues = {"HOBBY","STUDY","SPORTS"}) + private String category;src/main/java/project/flipnote/group/controller/GroupController.java (2)
14-14: 미사용 import 제거RequestParam이 사용되지 않습니다. 정리해 주세요.
-import org.springframework.web.bind.annotation.RequestParam;
87-96: Swagger 문서화 보강 제안기존 초대 조회 컨트롤러처럼 Docs 인터페이스를 분리하고 @operation(summary=..., security=...)를 부여하면 일관성과 가독성이 좋아집니다.
예시(새 파일, 참고용):
@Tag(name = "Group Query", description = "그룹 조회 API") public interface GroupQueryControllerDocs { @Operation(summary = "그룹 전체 조회", security = {@SecurityRequirement(name = "access-token")}) ResponseEntity<CursorPagingResponse<GroupInfo>> findGroup(AuthPrinciple authPrinciple, GroupListRequest req); }컨트롤러에 implements 추가 후 메서드 시그니처 동일 유지.
src/main/java/project/flipnote/group/service/GroupService.java (3)
47-48: 고정 SIZE 상수 제거 또는 최소화findGroup에서 요청 size를 사용하도록 수정하면 SIZE 상수는 의미가 약해집니다. 상수는 최대 허용치(예: MAX_SIZE=30) 정의로 용도 변경하거나 제거를 권장합니다. 실제 상한은 DTO에 @max(30)로 이미 보장됩니다.
- private static final int SIZE = 10; + // Optional: 필요 시 기본값만 유지하고 실제 계산에는 req.getSize() 사용 + private static final int DEFAULT_SIZE = 10;
345-355: 카테고리 변환/검증 로직 적절 — 대소문자 무시 처리 👍입력 공백/대소문자 처리를 커버하고, 잘못된 값에 대해 BizException을 명확히 던집니다. OpenAPI allowableValues와 병행 시 UX가 더 좋아집니다.
241-247: 메서드 단순화로 가독성 개선불리언 반환을 단순화 가능합니다.
- if (count > 0) { - return false; - } - return true; + return count == 0;
📜 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 (3)
src/main/java/project/flipnote/group/controller/GroupController.java(2 hunks)src/main/java/project/flipnote/group/model/GroupListRequest.java(1 hunks)src/main/java/project/flipnote/group/service/GroupService.java(12 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/project/flipnote/group/model/GroupListRequest.java (3)
src/main/java/project/flipnote/common/model/request/CursorPagingRequest.java (1)
Getter(13-60)src/main/java/project/flipnote/notification/model/NotificationListRequest.java (1)
Getter(11-24)src/main/java/project/flipnote/common/model/request/PagingRequest.java (1)
Getter(12-42)
src/main/java/project/flipnote/group/service/GroupService.java (2)
src/test/java/project/flipnote/group/service/GroupServiceTest.java (4)
Test(362-403)groupService(433-433)ExtendWith(52-475)Test(184-208)src/main/java/project/flipnote/group/service/GroupPolicyService.java (1)
Service(18-54)
src/main/java/project/flipnote/group/controller/GroupController.java (2)
src/main/java/project/flipnote/group/controller/GroupInvitationQueryController.java (1)
RequiredArgsConstructor(21-50)src/main/java/project/flipnote/group/controller/docs/GroupInvitationQueryControllerDocs.java (2)
Operation(17-22)Tag(14-29)
🔇 Additional comments (2)
src/main/java/project/flipnote/group/service/GroupService.java (2)
296-299: 소유자 멤버 선삭제 후 그룹 삭제 순서 적절FK 제약 충돌을 피하는 안전한 순서입니다. 트랜잭션 경계(@transactional)도 있어 일관성 확보에 도움 됩니다.
65-72: exists 기반의 경량 유저 검증으로 조회 비용 최적화findGroup에서 전체 엔티티 로딩 대신 exists 체크로 바꾼 점 좋습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/project/flipnote/group/service/GroupService.java (1)
325-343: 커서 페이징 저장소 계약 재확인 필요(pageSize+1, 정렬/커서 조건 일치)서비스는 groups.size() > pageSize를 전제로 hasNext를 계산합니다. 이는 저장소가 pageSize+1건을 반환할 때만 정확합니다. 또한 orderBy(id desc)라면 커서 비교는 id < lastId가 되어야 합니다.
검증 스크립트:
#!/bin/bash # 1) 저장소 시그니처 확인 rg -n -C2 "interface\s+GroupRepositoryCustom\b" src/main/java rg -n -C2 "findAllByCursor\\(" src/main/java/project/flipnote/group/repository # 2) 구현부의 limit(pageSize + 1) 및 커서 비교(id < lastId) 확인 rg -n -C3 "class .*GroupRepositoryImpl" src/main/java/project/flipnote/group/repository rg -n -C3 "findAllByCursor\\(" src/main/java/project/flipnote/group/repository rg -n -C3 "\\.limit\\(" src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java rg -n -C3 "orderBy\\(.*id\\.desc" src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java rg -n -C3 "\\.lt\\(lastId\\)" src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java
🧹 Nitpick comments (5)
src/main/java/project/flipnote/group/service/GroupService.java (5)
47-48: 페이지 기본값 상수의 목적 명확화(이름/용도 정리 제안)현재 SIZE(=10)는 본문에서 직접 사용되지 않아 의미가 불분명합니다. 아래처럼 “기본 페이지 사이즈” 역할이 드러나도록 이름을 바꾸고, findGroup에서 유효하지 않은 size 입력의 fallback으로 활용하는 편이 가독성과 유지보수성에 좋습니다.
- private static final int SIZE = 10; + // 기본 페이지 사이즈 (요청 size가 0 이하일 때 fallback) + private static final int DEFAULT_PAGE_SIZE = 10;그리고 findGroup 내부에서 DEFAULT_PAGE_SIZE를 활용하도록 함께 변경해 주세요(아래 관련 코멘트 참조).
65-72: 주석 의미 보강(“유저 정보 조회” → “유저 존재 검증”)메서드 목적은 존재 검증이며 실제 엔티티를 반환하지 않습니다. 주석을 명확히 해 주세요.
- /* - 유저 정보 조회 - */ + /* + 유저 존재 검증 (DB 존재 여부만 확인) + */ public void validateUser(AuthPrinciple authPrinciple) { if(!userProfileRepository.existsByIdAndStatus(authPrinciple.userId(), UserStatus.ACTIVE)) { throw new BizException(UserErrorCode.USER_NOT_FOUND); } }
228-231: 주석 오탈자 수정(“권환” → “권한”)주석 내 ‘유저 권환 조회’ 표기가 반복됩니다. 오탈자만 정리해 주세요.
- //5. 유저 권환 조회 + //5. 유저 권한 조회 ... - //4. 유저 권환 조회 + //4. 유저 권한 조회Also applies to: 286-289
281-297: Group-멤버 및 그룹 삭제 시 Cascade/Soft Delete 설정 확인 필요현재 오너 멤버 삭제 후 그룹 삭제 순서는 외래키 제약 상 안전하지만, JPA 매핑 구조와 Soft Delete 전략의 일관성 측면에서 아래 사항을 꼭 검토해주세요.
- Group.java
@SQLDelete/@SQLRestriction로 소프트 삭제 처리됨- 멤버(
GroupMember)와의 양방향@OneToMany매핑이 없어 JPA 레벨에서cascade = REMOVE또는orphanRemoval = true가 적용되지 않음- GroupMember.java
@ManyToOne(fetch = FetchType.LAZY)단방향 매핑groupMemberRepository.delete()호출 시 물리 삭제됨- DB 스키마
group_members.group_idFK 에 ON DELETE CASCADE 옵션이 있는지 확인- Soft Delete 전략 일관성
- 그룹은 소프트 삭제, 멤버는 물리 삭제 → 도메인 요구사항에 따라 멤버에도 소프트 삭제를 적용하거나, 물리 삭제 전략으로 통일할지 검토
옵션(선택적 리팩토링)
public class Group extends BaseEntity { // 기존 어노테이션... + @OneToMany(mappedBy = "group", cascade = CascadeType.REMOVE, orphanRemoval = true) + private Set<GroupMember> members = new HashSet<>(); }
241-247: 불리언 반환 간소화 및 네이밍 일관성 개선 제안아래 사항을 반영하면 가독성과 유지보수성이 향상됩니다:
GroupService.checkUserNotExistInGroup(...)의 불필요한 if-else를 제거하고 단일 return 문으로 간소화- 메서드 의도를 보다 명확히 드러내도록 네이밍 개선 (예:
isOnlyMember,hasNoOtherMembersExceptOwner등)GroupMemberRepository의 메서드 명명 컨벤션 통일
• 현재countByGroup_idAndUser_idNotvsfindByGroup_IdAndUser_Id로 케이스가 일치하지 않음
•countByGroup_IdAndUser_IdNot혹은 타입 세이프한countByGroupAndUserNot(Group group, UserProfile user)사용 검토- private boolean checkUserNotExistInGroup(UserProfile user, Group group) { - long count = groupMemberRepository.countByGroup_idAndUser_idNot(group.getId(), user.getId()); - if (count > 0) { - return false; - } - return true; - } + private boolean isOnlyMember(UserProfile owner, Group group) { + long count = groupMemberRepository.countByGroup_idAndUser_idNot(group.getId(), owner.getId()); + return count == 0; + }
- 레포지토리 메서드명 변경 예시
// 언더스코어 명명 일관화 long countByGroup_IdAndUser_IdNot(Long groupId, Long userId); // 또는 타입 세이프 방식 long countByGroupAndUserNot(Group group, UserProfile user);
📜 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 (1)
src/main/java/project/flipnote/group/service/GroupService.java(12 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/project/flipnote/group/service/GroupService.java (2)
src/test/java/project/flipnote/group/service/GroupServiceTest.java (3)
Test(362-403)ExtendWith(52-475)groupService(433-433)src/main/java/project/flipnote/group/service/GroupPolicyService.java (1)
Service(18-54)
🔇 Additional comments (7)
src/main/java/project/flipnote/group/service/GroupService.java (7)
12-15: 새 API/모델 도입 임포트 정합성 확인 완료CursorPagingResponse, Category, GroupInfo, GroupListRequest 임포트 추가는 변경 내용과 일치하며 사용처도 적절합니다.
Also applies to: 26-28
59-63: 유저 조회 헬퍼 메서드 도입 적절인증 주체에서 활성 사용자만 조회하도록 캡슐화한 점 좋습니다. 다수 호출부에서 일관된 예외 처리 흐름을 유지할 수 있습니다.
116-118: 그룹 생성 시 사용자 조회 흐름 일원화 LGTMcreate()에서 getUser() 재사용으로 예외/검증 플로우가 통일되었습니다.
216-218: 그룹 수정 시 사용자 조회 흐름 일원화 LGTMchangeGroup()도 동일한 조회/예외 규약을 따릅니다.
266-266: 상세 조회 시 사용자 조회 흐름 일원화 LGTM
314-315: 멤버 조회 API의 사용자 검증 흐름 일관성 LGTM
345-355: 카테고리 변환 로직 LGTM, 예외 처리 일관성 좋음대소문자/공백 허용 및 잘못된 값에 대한 도메인 예외 처리 흐름이 명확합니다.
📝 변경 내용
✅ 체크리스트
💬 기타 참고 사항
Summary by CodeRabbit