Feat: 특정 그룹의 카드셋 조회 API 구현 #51
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough그룹별 카드셋 검색 API가 추가되었습니다. 컨트롤러 엔드포인트, 서비스 메서드, 저장소 인터페이스/구현이 groupId 기반 필터를 수용하도록 확장되고, 그룹 존재 검증이 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as GroupCardSetController
participant Service as CardSetService
participant Repo as CardSetRepositoryCustomImpl
Client->>Controller: GET /groups/{groupId}/cardsets?keyword=&category=&page=
Controller->>Service: getCardSets(groupId, CardSetSearchRequest)
Service->>Service: groupService.validateGroupExists(groupId)
Service->>Repo: searchByGroupIdAndNameContainingAndCategory(groupId,name,category,pageable)
rect rgb(220, 240, 220)
Note over Repo: 동적 필터 적용\n(groupId, name, category, public 여부)\n메타조인 및 페이징/정렬 처리
Repo-->>Service: Page<CardSetInfo>
end
Service->>Service: map -> CardSetSummaryResponse, build PagingResponse
Service-->>Controller: PagingResponse<CardSetSummaryResponse>
Controller-->>Client: 200 OK + 페이징 응답
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/project/flipnote/cardset/controller/GroupCardSetController.java(3 hunks)src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustom.java(1 hunks)src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java(4 hunks)src/main/java/project/flipnote/cardset/service/CardSetService.java(1 hunks)src/main/java/project/flipnote/common/exception/GlobalExceptionHandler.java(1 hunks)
🔇 Additional comments (4)
src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustom.java (1)
21-27: LGTM!메서드 시그니처가 기존 패턴을 잘 따르고 있으며, 그룹별 검색 기능을 적절하게 정의하고 있습니다.
src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java (2)
92-92: LGTM!기존 메서드가 리팩토링된 필터 빌더를 올바르게 사용하고 있으며,
null을 전달하여 그룹 필터링을 비활성화하는 방식이 적절합니다.Also applies to: 110-110
225-236: LGTM!
groupIdEquals헬퍼 메서드와 리팩토링된 필터 빌더가 기존 패턴을 잘 따르고 있으며, 널 값을 적절하게 처리하고 있습니다.src/main/java/project/flipnote/cardset/controller/GroupCardSetController.java (1)
7-7: LGTM!새로운 엔드포인트에 필요한 import들이 올바르게 추가되었습니다.
Also applies to: 19-20, 25-25
Comment on lines
+69
to
+77
| @GetMapping | ||
| public ResponseEntity<PagingResponse<CardSetSummaryResponse>> getCardSets( | ||
| @PathVariable("groupId") Long groupId, | ||
| @Valid @ModelAttribute CardSetSearchRequest req | ||
| ) { | ||
| PagingResponse<CardSetSummaryResponse> res = cardSetService.getCardSets(groupId, req); | ||
|
|
||
| return ResponseEntity.ok(res); | ||
| } |
There was a problem hiding this comment.
인증 및 그룹 유효성 검증을 추가하세요.
이 엔드포인트에는 다음과 같은 보안 문제가 있습니다:
- 인증 누락: 다른 엔드포인트들(
createCardSet,getCardSet,updateCardSet)과 달리@AuthenticationPrincipal어노테이션이 없어 인증되지 않은 사용자도 접근 가능합니다. - 그룹 존재 검증 누락: 존재하지 않는
groupId에 대한 검증이 없습니다. - 접근 권한 검증 필요 여부 불명확: 공개 카드셋만 반환한다면 인증이 불필요할 수 있지만, 그룹 멤버에게 비공개 카드셋도 보여주어야 한다면 접근 권한 검증이 필요합니다.
요구사항에 따라 다음 중 하나를 선택하여 적용하세요:
옵션 1: 인증된 사용자만 접근 가능하고 그룹 멤버는 비공개 카드셋도 조회 가능
@GetMapping
public ResponseEntity<PagingResponse<CardSetSummaryResponse>> getCardSets(
@PathVariable("groupId") Long groupId,
- @Valid @ModelAttribute CardSetSearchRequest req
+ @Valid @ModelAttribute CardSetSearchRequest req,
+ @AuthenticationPrincipal AuthPrinciple authPrinciple
) {
- PagingResponse<CardSetSummaryResponse> res = cardSetService.getCardSets(groupId, req);
+ PagingResponse<CardSetSummaryResponse> res = cardSetService.getCardSets(groupId, req, authPrinciple.userId());
return ResponseEntity.ok(res);
}옵션 2: 인증 없이 공개 카드셋만 조회 가능 (현재 구현과 유사)
@GetMapping
public ResponseEntity<PagingResponse<CardSetSummaryResponse>> getCardSets(
@PathVariable("groupId") Long groupId,
@Valid @ModelAttribute CardSetSearchRequest req
) {
+ // 그룹 존재 여부 검증은 서비스 레이어에서 수행되어야 함
PagingResponse<CardSetSummaryResponse> res = cardSetService.getCardSets(groupId, req);
return ResponseEntity.ok(res);
}서비스 레이어에서 그룹 존재 검증도 추가해야 합니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/project/flipnote/cardset/controller/GroupCardSetController.java
around lines 69-77, this GET handler lacks authentication and group
existence/permission checks; update it per one of the two allowed behaviors:
Option 1 — require authenticated user by adding @AuthenticationPrincipal to the
method signature and pass the principal to the service, then have the service
validate the group exists and that the caller is a group member so it returns
both public and private cardsets; Option 2 — keep it public but explicitly
validate the group exists in the service and ensure the service only returns
public cardsets; in either case move group existence validation into the service
layer and throw a proper 404 if the group is missing.
Comment on lines
136
to
208
| @Override | ||
| public Page<CardSetInfo> searchByGroupIdAndNameContainingAndCategory( | ||
| long groupId, | ||
| String name, | ||
| Category category, | ||
| Pageable pageable | ||
| ) { | ||
| List<OrderSpecifier<?>> orders = new ArrayList<>(); | ||
|
|
||
| boolean useMetadata = false; | ||
| boolean hasIdSort = false; | ||
| for (Sort.Order order : pageable.getSort()) { | ||
| CardSetSortField sortField = null; | ||
| try { | ||
| sortField = CardSetSortField.valueOf(order.getProperty()); | ||
| } catch (IllegalArgumentException iae) { | ||
| log.warn( | ||
| "Unknown sort property: {}. Valid values are {}", | ||
| order.getProperty(), Arrays.toString(CardSetSortField.values()), iae | ||
| ); | ||
| } | ||
| if (sortField == CardSetSortField.LIKE) { | ||
| orders.add(toOrderSpecifier(cardSetMetadata.likeCount, order)); | ||
| useMetadata = true; | ||
| } else if (sortField == CardSetSortField.BOOKMARK) { | ||
| orders.add(toOrderSpecifier(cardSetMetadata.bookmarkCount, order)); | ||
| useMetadata = true; | ||
| } else { | ||
| orders.add(toOrderSpecifier(cardSet.id, order)); | ||
| hasIdSort = true; | ||
| } | ||
| } | ||
|
|
||
| if (!hasIdSort) { | ||
| orders.add(cardSet.id.desc()); | ||
| } | ||
|
|
||
| JPAQuery<CardSetInfo> selectQuery = queryFactory | ||
| .select( | ||
| Projections.constructor( | ||
| CardSetInfo.class, | ||
| cardSet, | ||
| cardSet.group, | ||
| cardSet.name, | ||
| cardSet.category, | ||
| cardSet.hashtag, | ||
| cardSet.imageUrl, | ||
| imageRef.id | ||
| )) | ||
| .from(cardSet) | ||
| .where(buildCardSetSearchFilterConditions(groupId, name, category)) | ||
| .leftJoin(imageRef) | ||
| .on(imageRef.referenceType.eq(ReferenceType.CARD_SET) | ||
| .and(imageRef.referenceId.eq(cardSet.id))); | ||
|
|
||
| if (useMetadata) { | ||
| selectQuery.leftJoin(cardSetMetadata).on(cardSet.id.eq(cardSetMetadata.id)); | ||
| } | ||
|
|
||
| List<CardSetInfo> content = selectQuery | ||
| .orderBy(orders.toArray(new OrderSpecifier[0])) | ||
| .offset(pageable.getOffset()) | ||
| .limit(pageable.getPageSize()) | ||
| .fetch(); | ||
|
|
||
| Long total = queryFactory | ||
| .select(cardSet.count()) | ||
| .from(cardSet) | ||
| .where(buildCardSetSearchFilterConditions(groupId, name, category)) | ||
| .fetchOne(); | ||
|
|
||
| return new PageImpl<>(content, pageable, total != null ? total : 0L); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
중복 코드를 제거하여 단일 메서드로 통합하세요.
이 메서드는 searchByNameContainingAndCategory (라인 43-114)와 거의 동일하며, 유일한 차이점은 라인 186에서 groupId를 전달하는 것입니다. 이러한 중복은 유지보수성을 크게 저하시킵니다.
다음과 같이 리팩토링하는 것을 권장합니다:
@Override
public Page<CardSetInfo> searchByNameContainingAndCategory(
String name,
Category category,
Pageable pageable
) {
+ return searchByGroupIdAndNameContainingAndCategory(null, name, category, pageable);
+ }
+
+ @Override
+ public Page<CardSetInfo> searchByGroupIdAndNameContainingAndCategory(
+ Long groupId,
+ String name,
+ Category category,
+ Pageable pageable
+ ) {
List<OrderSpecifier<?>> orders = new ArrayList<>();
boolean useMetadata = false;
boolean hasIdSort = false;
for (Sort.Order order : pageable.getSort()) {
CardSetSortField sortField = null;
try {
sortField = CardSetSortField.valueOf(order.getProperty());
} catch (IllegalArgumentException iae) {
log.warn(
"Unknown sort property: {}. Valid values are {}",
order.getProperty(), Arrays.toString(CardSetSortField.values()), iae
);
}
if (sortField == CardSetSortField.LIKE) {
orders.add(toOrderSpecifier(cardSetMetadata.likeCount, order));
useMetadata = true;
} else if (sortField == CardSetSortField.BOOKMARK) {
orders.add(toOrderSpecifier(cardSetMetadata.bookmarkCount, order));
useMetadata = true;
} else {
orders.add(toOrderSpecifier(cardSet.id, order));
hasIdSort = true;
}
}
if (!hasIdSort) {
orders.add(cardSet.id.desc());
}
JPAQuery<CardSetInfo> selectQuery = queryFactory
.select(
Projections.constructor(
CardSetInfo.class,
cardSet,
cardSet.group,
cardSet.name,
cardSet.category,
cardSet.hashtag,
cardSet.imageUrl,
imageRef.id
))
.from(cardSet)
- .where(buildCardSetSearchFilterConditions(null, name, category))
+ .where(buildCardSetSearchFilterConditions(groupId, name, category))
.leftJoin(imageRef)
.on(imageRef.referenceType.eq(ReferenceType.CARD_SET)
.and(imageRef.referenceId.eq(cardSet.id)));
if (useMetadata) {
selectQuery.leftJoin(cardSetMetadata).on(cardSet.id.eq(cardSetMetadata.id));
}
List<CardSetInfo> content = selectQuery
.orderBy(orders.toArray(new OrderSpecifier[0]))
.offset(pageable.getOffset())
.limit(pageable.getPageSize())
.fetch();
Long total = queryFactory
.select(cardSet.count())
.from(cardSet)
- .where(buildCardSetSearchFilterConditions(null, name, category))
+ .where(buildCardSetSearchFilterConditions(groupId, name, category))
.fetchOne();
return new PageImpl<>(content, pageable, total != null ? total : 0L);
}
-
- @Override
- public Page<CardSetInfo> searchByGroupIdAndNameContainingAndCategory(
- long groupId,
- String name,
- Category category,
- Pageable pageable
- ) {
- List<OrderSpecifier<?>> orders = new ArrayList<>();
-
- boolean useMetadata = false;
- boolean hasIdSort = false;
- for (Sort.Order order : pageable.getSort()) {
- CardSetSortField sortField = null;
- try {
- sortField = CardSetSortField.valueOf(order.getProperty());
- } catch (IllegalArgumentException iae) {
- log.warn(
- "Unknown sort property: {}. Valid values are {}",
- order.getProperty(), Arrays.toString(CardSetSortField.values()), iae
- );
- }
- if (sortField == CardSetSortField.LIKE) {
- orders.add(toOrderSpecifier(cardSetMetadata.likeCount, order));
- useMetadata = true;
- } else if (sortField == CardSetSortField.BOOKMARK) {
- orders.add(toOrderSpecifier(cardSetMetadata.bookmarkCount, order));
- useMetadata = true;
- } else {
- orders.add(toOrderSpecifier(cardSet.id, order));
- hasIdSort = true;
- }
- }
-
- if (!hasIdSort) {
- orders.add(cardSet.id.desc());
- }
-
- JPAQuery<CardSetInfo> selectQuery = queryFactory
- .select(
- Projections.constructor(
- CardSetInfo.class,
- cardSet,
- cardSet.group,
- cardSet.name,
- cardSet.category,
- cardSet.hashtag,
- cardSet.imageUrl,
- imageRef.id
- ))
- .from(cardSet)
- .where(buildCardSetSearchFilterConditions(groupId, name, category))
- .leftJoin(imageRef)
- .on(imageRef.referenceType.eq(ReferenceType.CARD_SET)
- .and(imageRef.referenceId.eq(cardSet.id)));
-
- if (useMetadata) {
- selectQuery.leftJoin(cardSetMetadata).on(cardSet.id.eq(cardSetMetadata.id));
- }
-
- List<CardSetInfo> content = selectQuery
- .orderBy(orders.toArray(new OrderSpecifier[0]))
- .offset(pageable.getOffset())
- .limit(pageable.getPageSize())
- .fetch();
-
- Long total = queryFactory
- .select(cardSet.count())
- .from(cardSet)
- .where(buildCardSetSearchFilterConditions(groupId, name, category))
- .fetchOne();
-
- return new PageImpl<>(content, pageable, total != null ? total : 0L);
- }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java
around lines 136 to 208, this method is almost identical to
searchByNameContainingAndCategory (lines 43-114) except it passes groupId;
refactor by extracting the shared query-building/pagination logic into a single
private helper (e.g., searchCardSetsInternal) that accepts an optional/groupId
parameter (or Long groupId that may be null), the name, category, pageable and
returns Page<CardSetInfo>; move common logic for sort parsing, metadata join,
projection, ordering, offset/limit and total count into that helper, and update
both public methods to call it (one passing the groupId, the other passing
null), preserving existing behavior (conditional metadata join, id fallback
sort, and where clause built using the passed groupId).
Comment on lines
329
to
347
|
|
||
| /** | ||
| * 특정 그룹의 카드셋 목록을 페이지 단위로 조회 | ||
| * | ||
| * @param groupId 조회할 그룹의 ID | ||
| * @param req 조회 조건 및 페이징 정보를 포함한 요청 DTO | ||
| * @return 페이지 단위로 조회된 카드셋 목록 | ||
| * @author 윤정환 | ||
| */ | ||
| public PagingResponse<CardSetSummaryResponse> getCardSets(long groupId, CardSetSearchRequest req) { | ||
| // TODO: Projection 튜닝 필요 | ||
| Page<CardSetInfo> cardSetPage = cardSetRepository.searchByGroupIdAndNameContainingAndCategory( | ||
| groupId, req.getKeyword(), Category.from(req.getCategory()), req.getPageRequest() | ||
| ); | ||
|
|
||
| Page<CardSetSummaryResponse> res = cardSetPage.map(CardSetSummaryResponse::from); | ||
|
|
||
| return PagingResponse.from(res); | ||
| } |
There was a problem hiding this comment.
그룹 유효성 검증 및 접근 제어 로직을 추가하세요.
이 메서드에는 다음과 같은 문제가 있습니다:
- 그룹 존재 검증 누락: 라인 76-80의
findGroup헬퍼를 사용하여 그룹이 존재하는지 확인해야 합니다. - 타입 불일치: 파라미터가 primitive
long이지만 컨트롤러는Long을 전달합니다.Long으로 변경하는 것이 일관성 있습니다. - 접근 제어 불명확: 현재 구현은 repository에서
publicVisible만 필터링하지만, 그룹 멤버는 비공개 카드셋도 조회할 수 있어야 하는지 명확하지 않습니다.
다음과 같이 개선하는 것을 권장합니다:
최소한 그룹 검증은 반드시 추가:
- public PagingResponse<CardSetSummaryResponse> getCardSets(long groupId, CardSetSearchRequest req) {
+ public PagingResponse<CardSetSummaryResponse> getCardSets(Long groupId, CardSetSearchRequest req) {
// TODO: Projection 튜닝 필요
+ findGroup(groupId); // 그룹 존재 검증
Page<CardSetInfo> cardSetPage = cardSetRepository.searchByGroupIdAndNameContainingAndCategory(
groupId, req.getKeyword(), Category.from(req.getCategory()), req.getPageRequest()
);
Page<CardSetSummaryResponse> res = cardSetPage.map(CardSetSummaryResponse::from);
return PagingResponse.from(res);
}그룹 멤버가 비공개 카드셋도 조회해야 한다면:
- public PagingResponse<CardSetSummaryResponse> getCardSets(long groupId, CardSetSearchRequest req) {
+ public PagingResponse<CardSetSummaryResponse> getCardSets(Long groupId, CardSetSearchRequest req, Long userId) {
// TODO: Projection 튜닝 필요
+ Group group = findGroup(groupId); // 그룹 존재 검증
+
Page<CardSetInfo> cardSetPage = cardSetRepository.searchByGroupIdAndNameContainingAndCategory(
groupId, req.getKeyword(), Category.from(req.getCategory()), req.getPageRequest()
);
- Page<CardSetSummaryResponse> res = cardSetPage.map(CardSetSummaryResponse::from);
+ // userId가 null이면 공개 카드셋만, 그룹 멤버면 비공개 카드셋도 포함
+ Page<CardSetSummaryResponse> res = cardSetPage
+ .map(info -> {
+ boolean isGroupMember = userId != null && existGroupMember(group, validateUser(userId));
+ if (info.cardSet().isPublicVisible() || isGroupMember) {
+ return CardSetSummaryResponse.from(info);
+ }
+ return null;
+ })
+ .filter(Objects::nonNull);
return PagingResponse.from(res);
}구체적인 접근 제어 요구사항을 확인하고 적절한 구현을 선택하세요.
📝 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.
Suggested change
| /** | |
| * 특정 그룹의 카드셋 목록을 페이지 단위로 조회 | |
| * | |
| * @param groupId 조회할 그룹의 ID | |
| * @param req 조회 조건 및 페이징 정보를 포함한 요청 DTO | |
| * @return 페이지 단위로 조회된 카드셋 목록 | |
| * @author 윤정환 | |
| */ | |
| public PagingResponse<CardSetSummaryResponse> getCardSets(long groupId, CardSetSearchRequest req) { | |
| // TODO: Projection 튜닝 필요 | |
| Page<CardSetInfo> cardSetPage = cardSetRepository.searchByGroupIdAndNameContainingAndCategory( | |
| groupId, req.getKeyword(), Category.from(req.getCategory()), req.getPageRequest() | |
| ); | |
| Page<CardSetSummaryResponse> res = cardSetPage.map(CardSetSummaryResponse::from); | |
| return PagingResponse.from(res); | |
| } | |
| /** | |
| * 특정 그룹의 카드셋 목록을 페이지 단위로 조회 | |
| * | |
| * @param groupId 조회할 그룹의 ID | |
| * @param req 조회 조건 및 페이징 정보를 포함한 요청 DTO | |
| * @return 페이지 단위로 조회된 카드셋 목록 | |
| * @author 윤정환 | |
| */ | |
| public PagingResponse<CardSetSummaryResponse> getCardSets(Long groupId, CardSetSearchRequest req) { | |
| // TODO: Projection 튜닝 필요 | |
| findGroup(groupId); // 그룹 존재 검증 | |
| Page<CardSetInfo> cardSetPage = cardSetRepository.searchByGroupIdAndNameContainingAndCategory( | |
| groupId, req.getKeyword(), Category.from(req.getCategory()), req.getPageRequest() | |
| ); | |
| Page<CardSetSummaryResponse> res = cardSetPage.map(CardSetSummaryResponse::from); | |
| return PagingResponse.from(res); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/project/flipnote/cardset/service/CardSetService.java around
lines 329 to 347, add group existence validation and adjust parameter type and
access control: change the groupId parameter from primitive long to Long to
match the controller, call the existing findGroup helper (lines ~76-80) to
verify the group exists and throw the same not-found exception if absent, and
modify the repository query logic to apply access control (when the caller is a
group member include private/non-public cardsets, otherwise filter by
publicVisible only); preserve the pagination and Category.from conversion and
ensure the method returns PagingResponse.from(mappedPage).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/main/java/project/flipnote/cardset/service/CardSetService.java (2)
338-338: 파라미터 타입을Long으로 변경하는 것을 권장합니다.현재 primitive
long을 사용하고 있으나, 다른 메서드들(예:findGroup,getCardSet)은 래퍼 타입Long을 사용합니다. 일관성과 null-safety를 위해Long으로 변경하는 것이 좋습니다.- public PagingResponse<CardSetSummaryResponse> getCardSets(long groupId, CardSetSearchRequest req) { + public PagingResponse<CardSetSummaryResponse> getCardSets(Long groupId, CardSetSearchRequest req) {
330-349: 그룹 멤버의 비공개 카드셋 접근 권한을 고려하세요.현재 구현은 모든 사용자에게 공개(
publicVisible) 카드셋만 반환합니다. 그룹 멤버가 해당 그룹의 비공개 카드셋도 조회할 수 있어야 하는지 요구사항을 확인해 주세요.만약 그룹 멤버가 비공개 카드셋도 볼 수 있어야 한다면, 사용자 ID를 파라미터로 받아
groupMemberRepository를 통해 멤버십을 확인하고, 멤버인 경우 비공개 카드셋도 포함하도록 필터링 로직을 조정해야 합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/project/flipnote/cardset/model/CardSetSummaryResponse.java(0 hunks)src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustom.java(1 hunks)src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java(4 hunks)src/main/java/project/flipnote/cardset/service/CardSetService.java(3 hunks)src/main/java/project/flipnote/group/service/GroupService.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/project/flipnote/cardset/model/CardSetSummaryResponse.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustom.java
🔇 Additional comments (2)
src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java (2)
48-50: 중복 제거 리팩토링 잘 적용되었습니다!이전 리뷰에서 제안된 대로
searchByGroupIdAndNameContainingAndCategory에null을 전달하여 중복 코드를 제거했습니다. 이는 DRY 원칙을 준수하며 유지보수성을 크게 향상시킵니다.
161-163: LGTM! 일관된 패턴으로 구현되었습니다.
groupIdEquals헬퍼 메서드는 기존의nameContains,categoryEquals와 동일한 패턴을 따르며, null-safe하게 구현되어 있습니다.Also applies to: 165-172
Comment on lines
+52
to
+70
| public List<CardSetInfo> findAllByIdWithImageRefId(Set<Long> cardSets) { | ||
| return queryFactory.select( | ||
| Projections.constructor( | ||
| CardSetInfo.class, | ||
| cardSet, | ||
| cardSet.group, | ||
| cardSet.name, | ||
| cardSet.category, | ||
| cardSet.hashtag, | ||
| cardSet.imageUrl, | ||
| imageRef.id | ||
| )) | ||
| .from(cardSet) | ||
| .where(cardSet.id.in(cardSets)) | ||
| .leftJoin(imageRef) | ||
| .on(imageRef.referenceType.eq(ReferenceType.CARD_SET) | ||
| .and(imageRef.referenceId.eq(cardSet.id))) | ||
| .fetch(); | ||
| } |
There was a problem hiding this comment.
@OverRide 어노테이션이 누락되었습니다.
이 메서드가 인터페이스를 구현하는 경우 @Override 어노테이션을 추가하여 명시적으로 표시해야 합니다.
+ @Override
public List<CardSetInfo> findAllByIdWithImageRefId(Set<Long> cardSets) {📝 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.
Suggested change
| public List<CardSetInfo> findAllByIdWithImageRefId(Set<Long> cardSets) { | |
| return queryFactory.select( | |
| Projections.constructor( | |
| CardSetInfo.class, | |
| cardSet, | |
| cardSet.group, | |
| cardSet.name, | |
| cardSet.category, | |
| cardSet.hashtag, | |
| cardSet.imageUrl, | |
| imageRef.id | |
| )) | |
| .from(cardSet) | |
| .where(cardSet.id.in(cardSets)) | |
| .leftJoin(imageRef) | |
| .on(imageRef.referenceType.eq(ReferenceType.CARD_SET) | |
| .and(imageRef.referenceId.eq(cardSet.id))) | |
| .fetch(); | |
| } | |
| @Override | |
| public List<CardSetInfo> findAllByIdWithImageRefId(Set<Long> cardSets) { | |
| return queryFactory.select( | |
| Projections.constructor( | |
| CardSetInfo.class, | |
| cardSet, | |
| cardSet.group, | |
| cardSet.name, | |
| cardSet.category, | |
| cardSet.hashtag, | |
| cardSet.imageUrl, | |
| imageRef.id | |
| )) | |
| .from(cardSet) | |
| .where(cardSet.id.in(cardSets)) | |
| .leftJoin(imageRef) | |
| .on(imageRef.referenceType.eq(ReferenceType.CARD_SET) | |
| .and(imageRef.referenceId.eq(cardSet.id))) | |
| .fetch(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java
around lines 52 to 70, the method findAllByIdWithImageRefId is missing the
@Override annotation; add the @Override annotation immediately above the method
signature to explicitly indicate it implements the interface method (no other
logic changes required).
Comment on lines
+449
to
+460
| /** | ||
| * 지정된 그룹 ID가 존재하는지 검사합니다. | ||
| * | ||
| * @param groupId 존재 여부를 확인할 그룹의 ID | ||
| * @throws BizException 그룹이 존재하지 않을 경우 발생 | ||
| * @author 윤정환 | ||
| */ | ||
| public void validateGroupExists(Long groupId) { | ||
| if (!groupRepository.existsById(groupId)) { | ||
| throw new BizException(GroupErrorCode.GROUP_NOT_FOUND); | ||
| } | ||
| } |
There was a problem hiding this comment.
소프트 삭제된 그룹에 대한 검증이 누락되었습니다.
이 메서드는 existsById를 사용하여 deletedAt을 확인하지 않습니다. 반면 기존의 validateGroup 메서드(Line 111-115)는 existsByIdAndDeletedAtIsNull을 사용하여 삭제된 그룹을 필터링합니다. 이러한 불일치로 인해 소프트 삭제된 그룹이 유효한 것으로 검증될 수 있습니다.
다음과 같이 수정하세요:
public void validateGroupExists(Long groupId) {
- if (!groupRepository.existsById(groupId)) {
+ if (!groupRepository.existsByIdAndDeletedAtIsNull(groupId)) {
throw new BizException(GroupErrorCode.GROUP_NOT_FOUND);
}
}🤖 Prompt for AI Agents
In src/main/java/project/flipnote/group/service/GroupService.java around lines
449 to 460, the validateGroupExists method uses groupRepository.existsById which
ignores soft-deleted rows; replace the existence check with the same
soft-delete-aware query used in validateGroup (e.g.,
groupRepository.existsByIdAndDeletedAtIsNull(groupId)) and throw
BizException(GroupErrorCode.GROUP_NOT_FOUND) when that returns false so
soft-deleted groups are treated as not found.
stoneTiger0912
approved these changes
Nov 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝 변경 내용
✅ 체크리스트
💬 기타 참고 사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 픽스 / 개선
스타일